felixguendling/cista

Cista::Variant serialization issue above C++17

Closed this issue · 7 comments

Hi Felix!

It's been a while since my last interaction with Cista.... so many new features since, my congrats!

To the point... :)
I have switched lately my project to the VS "/std:c++latest" and "/std:c++20" (both: 2019 and 2022 preview) and since that I noticed that this serialize(...) usage:

origin->apply([&](auto&& t) {
serialize(c, &t, pos + cista_member_offset(Type, storage_));
});

produces hard error: "Error C1001 Internal compiler error".

I know, I know that Cista is C++17 project... but since everything else just works(!) and only this one(!) particular error occurs (even during clean single-header build/tests) I though it is worth mentioning here.

FYI: I did all the preliminary use-case analysis, all of them - even simple tryout of cista::variant serialization leads to "this".

I will investigate it further on my own (right now)... but I'm silently counting on your help here - even it is outside the target (C++17).

Tracking: dumb removing cista_member_offset(Type, storage_) from equation helps, no error!

Tracking: dumb moving cista_member_offset calculation outside the lambda, works!

Solution

auto offset = cista_member_offset(Type, storage_);
origin->apply( [&](auto&& t) { serialize(c, &t, pos + offset); } );

...looking for the premises why this is happening above C++17

Thank you very much for your report. I don't think it's reasonable to try to reason about the "Why" in case of internal compiler errors as it's internal and there is no source code available for the compiler. So your approach trying out different versions of the same concept would also been my go-to approach.

Solution

auto offset = cista_member_offset(Type, storage_);
origin->apply( [&](auto&& t) { serialize(c, &t, pos + offset); } );

Great!

It's of course my goal to support C++20 as well as upcoming C++ standards with Cista (maybe as long as there is no useful cross-platform serialization support in the language). So your feedback/solution is much appreciated.

Would you like to make a PR? Otherwise, I can also integrate the changes. It probably makes sense to add CI checks with C++20, too.

I am away from my workstation right now... so could you please integrate this on your own and close the issue?
Next time (in such a case) I will provide PR.