felixguendling / cista

Cista is a simple, high-performance, zero-copy C++ serialization & reflection library.
https://cista.rocks
MIT License
1.82k stars 118 forks source link

How to make cista::get_if<Type> works? #98

Closed ChemistAion closed 3 years ago

ChemistAion commented 3 years ago

I have this little fellow here:

using Variant = CISTA::variant
    <
    bool,
    std::int64_t
    >;

Variant test{ true };

auto result = cista::get_if<bool>(test);

...and I am getting (on VS2019/VS2022prev, both C++20): "Error C2563 mismatch in formal parameter list" "Error C2568 '==': unable to resolve function overload" on: https://github.com/felixguendling/cista/blob/23875effbc4f3441304137515991dbcfbc52caed/include/cista/containers/variant.h#L380

I know that I could address bool by its index in Variant - namely 0, but how to make it happen with Type directly? I am trying to make this through cista::get_if implementations... test-case "variant_test.cc" helps me not.

[C2563] https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-2/compiler-error-c2563?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(C2563)%26rd%3Dtrue&view=msvc-160

[C2568] https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-2/compiler-error-c2568?f1url=%3FappId%3DDev16IDEF1%26l%3DEN-US%26k%3Dk(C2568)%26rd%3Dtrue&view=msvc-160

ChemistAion commented 3 years ago

I have better issue description from gcc, here below: https://github.com/felixguendling/cista/blob/23875effbc4f3441304137515991dbcfbc52caed/include/cista/containers/variant.h#L382

quote: [pointing on "=="] error: ISO C++ forbids comparison between pointer and integer [-fpermissive] return v.idx_ == &cista::index_of_type<T, Ts...> ? v.template as()

[pointing on "?"] error: operands to '?:' have different types 'bool' and 'std::nullptrt' return v.idx == &cista::index_of_type<T, Ts...> ? v.template as()

felixguendling commented 3 years ago

Thank you very much for your report.

I hope I fixed it (it's on master):

https://github.com/felixguendling/cista/blob/0848575608e3d810d858e3b03deaaa571230be35/include/cista/containers/variant.h#L379-L384

ChemistAion commented 3 years ago

Whoa! That was quick... :)

ChemistAion commented 3 years ago

Almost :)

It works until recurring reference container is added to the variant, i.e. CISTA::unique_ptr<Test> below:

struct Test;

using Variant = CISTA::variant
    <
    bool,
    std::int64_t,
    std::uint64_t,
    double,
    CISTA::string,
    CISTA::unique_ptr<int>,
    CISTA::unique_ptr<Test>
    >;

struct Test : public CISTA::vector<Variant>
{
};

////////////////////////////////////////////////////////////////////////////////

Variant test{ true };
auto result = cista::get_if<bool>(test);

Same errors on MSVS

felixguendling commented 3 years ago

This test case works for me:

TEST_CASE("variant get_if") {
  namespace CISTA = cista::offset;
  struct Test;

  using Variant =
      CISTA::variant<bool, std::int64_t, std::uint64_t, double, CISTA::string,
                     CISTA::unique_ptr<int>, CISTA::unique_ptr<Test>>;

  struct Test : public CISTA::vector<Variant> {};

  Variant test{true};
  auto const result = cista::get_if<bool>(test);
  REQUIRE(result != nullptr);
  CHECK(*result == true);

  test = CISTA::string{"hello test test test test"};
  auto const str = cista::get_if<CISTA::string>(test);
  REQUIRE(str != nullptr);
  CHECK(*str == "hello test test test test");
}

Checking to see if it breaks in GitHub Actions. da8da3a596550bfa22563d56dbcea04155fb42bf

Edit: seems the CI build works https://github.com/felixguendling/cista/actions/runs/1035547916

ChemistAion commented 3 years ago

btw: cista::get_if<>(const& v) should be also patched: https://github.com/felixguendling/cista/blob/da8da3a596550bfa22563d56dbcea04155fb42bf/include/cista/containers/variant.h#L402-L408

felixguendling commented 3 years ago

Should be fixed now by eb050ae7602dbf8d0b5f9c9023d8d41b2d2ce4b3

ChemistAion commented 3 years ago

Indeed, thank you.