cbeck88/visit_struct

"Intrusive" Syntax and member initialization

amallia opened this issue · 15 comments

Hi, is it possible to to member initialization while using the "Intrusive" Syntax?

Something like the following:

struct my_type {
  BEGIN_VISITABLES(my_type);
  VISITABLE(int, a, 0);
  VISITABLE(float, b, 0.0);
  VISITABLE(std::string, c);
  END_VISITABLES;
};

I agree, that would be great, and it is an oversight.

One thing that concerns me is, if the = initializer is implicit, then you cannot do certain things:

struct my_type {
   std::atomic<int> foo = 0; // doesn't compile: invoking deleted copy constructor of std::atomic<int>
};

struct my_type {
   std::atomic<int> foo{0}; // works
};

So I'd be tempted to make the with-initializer syntax like this:

struct my_type {
  BEGIN_VISITABLES(my_type);
  VISITABLE(int, a, =0);
  VISITABLE(float, b, =0.0);
  VISITABLE(std::string, c);
  VISITABLE(std::atomic<int>, d, {0});
  END_VISITABLES;
};

What do you think?

This should also work even if not ideal with std::atomic<int>.

struct my_type {
  BEGIN_VISITABLES(my_type);
  VISITABLE(int, a, 0);
  VISITABLE(float, b, 0.0);
  VISITABLE(std::string, c);
  VISITABLE(std::atomic<int>, d, {0});
  END_VISITABLES;
};

What do you think?

We can always think about having VISITABLE, VISITABLE_INIT and VISITABLE_LISTINIT.

struct my_type {
  BEGIN_VISITABLES(my_type);
  VISITABLE_INIT(int, a, 0);
  VISITABLE_INIT(float, b, 0.0);
  VISITABLE(std::string, c);
  VISITABLE_LIST(std::atomic<int>, d, {0});
  END_VISITABLES;
};

From my point of view the difference is "copy initialization" (using an = sign with the initializer e.g. int x = 5;) vs. "direct initialization" e.g. int x{5};)

It makes a difference if the class is not copyable or something

What would you think about:

  • VISITABLE_INIT takes a 3rd argument, and if it does, = is generated so that copy initialization is used
  • When direct initialization is needed, VISITABLE_DIRECT_INIT can be used, which is the same but doesn't put an =

I could also make it so that VISITABLE has an optional third argument which is the initializer, but it requires some preprocessor tricks to have optional arguments AFAIR, so it might harm the quality of error messages slightly when the macro is misused. I guess I can try it and see how the error messages look.

Yes! You totally got my point. I just called them differently: VISITABLE_INIT and VISITABLE_LISTINIT but the idea is the same. I actually like your naming better.

I wouldn't bother too much about merging VISITABLE_INIT into VISITABLE, lets just have 3 different macros so the programmer is aware of what is going on.

If you implement this I will be able to use it in my project! Thanks

Note: I believe this is stopping many from using it because the reason to use reflection is most likely because the struct is big, for the same reason you dont want to manually write a constructor that initialize with default values, but you cannot afford to leave the struct uninitialized...

I agree, thanks for bringing this up!

Could you let me know when the new feature is in place? Thank you

Yeah will do, sorry I've been super busy!

Just to let you know, I started working on this, going to make some tests and run on some more compilers just to make sure there's no problem :)

Thanks. Let me know if you want me to try it as well before you merge the change.

@cbeck88 I was looking for a simple standalone library to interface with some large structs (serialization, exposing to gui, ...) and finally came across your's, which looks fantastic, except for the missing initialization in the intrusive syntax. I'm sure you are probably busy, but did you manage to work on this?

Hey im really sorry, I just got married on Jan 4th. I will make time for this really soon! Someone once told me, love is the primary obstacle to many open source software projects... 😅

Haha, thanks for the quick reply and congratulations 🍾!

No worries, there are more important things in life than initializer lists 😆.

In the meantime this has been working well for me:

#define VISITABLE_INIT(TYPE, NAME, VALUE)                                                                        \
TYPE NAME = VALUE;                                                                                               \
struct VISIT_STRUCT_MAKE_MEMBER_NAME(NAME) :                                                                     \
  visit_struct::detail::member_ptr_helper<VISIT_STRUCT_CURRENT_TYPE,                                             \
                                          TYPE,                                                                  \
                                          &VISIT_STRUCT_CURRENT_TYPE::NAME>                                      \
{                                                                                                                \
  static VISIT_STRUCT_CONSTEXPR const ::visit_struct::detail::char_array<sizeof(#NAME)> & member_name() {        \
    return #NAME;                                                                                                \
  }                                                                                                              \
};                                                                                                               \
static inline ::visit_struct::detail::Append_t<VISIT_STRUCT_GET_REGISTERED_MEMBERS,                              \
                                               VISIT_STRUCT_MAKE_MEMBER_NAME(NAME)>                              \
  Visit_Struct_Get_Visitables__(::visit_struct::detail::Rank<VISIT_STRUCT_GET_REGISTERED_MEMBERS::size + 1>);    \
static_assert(true, "")

Also added VISITABLE_DIRECT_INIT, see #14.