[Draft]Add copy constructor for dynamic_format_arg_store

[spyridon97]

The goal of this PR is to implement properly what #2432 proposed but as detected in #2653, it produced undefined behavior, and that's why it was deleted in be51ee1.

It should be noted that this functionality is needed in https://github.com/Kitware/ParaView.

dynamic_format_arg_store has the following members which need to be copied over:

• detail::dynamic_arg_list dynamic_args_

• std::vector<basic_format_arg> data_

• std::vector<detail::named_arg_info<char_type>> named_info_

For this to work we need the following classes/structs/enums to have a copy constructor

• detail::dynamic_arg_list (Done in this PR)

• basic_format_arg which has the following member

  • detail::type (Default copy constructor is good enough)
  • detail::value (Done in this PR, but leads to wrong results)

• detail::named_arg_info<char_type> (Default copy constructor prints correct result when iterating over, but leads to segfault)

I would like your view on what i have already implemented and how could i go about implementing the remaining copy constructors if it's possible of course.

[spyridon97]

Note: copy_constructor, move_constructor tests fail for the time being.

The copy constructors that currently don't not work are detail::type and detail::named_arg_info<char_type>. That's because all the const char/void* variables when store.reset() is called become null since dynamic_args_list is deallocated.

How could I utilize the dynamic_args_list so i can have the correct values in basic_format_arg and detail::named_arg_info<char_type>?

[vitaut]

Could you remind why you want to add a copy constructor in the first place? In general dynamic_format_arg_store is supposed to be used for argument passing, not arbitrary management of user-defined objects. For the latter you should probably use a tuple.

[spyridon97]

Could you remind why you want to add a copy constructor in the first place? In general dynamic_format_arg_store is supposed to be used for argument passing, not arbitrary management of user-defined objects. For the latter you should probably use a tuple.

When ParaView is initialized we store in an argument list certain enviromental variables like os, username, hostname .etc that are available to the user in each formattable string. On top of that, they are certain views (like render view) that we want to add extra variable like time or title, and after we close the view, we want to remove those variables and use only the previous ones as a baseline for new ones.

To do that we are using a copy constructor and we have a stack of dynamic_format_arg_store and each time we push a new dynamic_format_arg_store we initialize using the previous top dynamic_format_arg_store. Once we finish using the new variables included in the top dynamic_format_arg_store, we pop the dynamic_format_arg_store from the stack.

Another approach to accomplish that is to be able to remove a variable from the dynamic_format_arg_store instead of using a copy constructor.

Both options seem like they can achieve the same thing, but i believe the second option seems easier to implement.

What do you think?

[vitaut]

What do you think?

Thanks for the explanation. I think it's a misuse of the dynamic_format_arg_store and you should use a real container instead only constructing the store at the time of doing the actual formatting.