gcc std::declval issues

[stephenberry]

gcc std::declval issues

[Ahajha]

Agreed, at least it's building now!

[Igisid]

I tried to fix the build error. It turned out something like this:

template <class T, class mptr_t>
constexpr decltype(auto)   member_check()
{
    using mptr_type = std::decay_t<mptr_t>;
    if constexpr (std::is_member_pointer_v<mptr_type>) {
        using Type = decltype(std::declval<T>());
        return Type{}.*mptr_type{};
    }
    else if constexpr (std::is_enum_v<std::decay_t<T>>) {
        return std::declval<T>();
    }
    else {  // is a lambda function
         using invoke = std::invoke_result_t<mptr_t, T>;
         return std::decay_t<invoke>{};
    }
}

The tests are passing. But when compiling a warning:
warning: returning reference to local temporary object [-Wreturn-stack-address]
I hope this warning can be fixed.

[stephenberry]

The problem with this is that it requires T, the exposed class, to be default constructible. But, we want to be able to create interfaces to non-default constructible types. This is the reason why we need std::declval rather than T{}, and a major reason why declval exists.

This...

using Type = decltype(std::declval<T>());
        return Type{}.*mptr_type{};

is the same as

  return T{}.*mptr_type{};

So, while this change currently passes tests, it really isn't a solution. But, thanks for looking into it. We'll keep trying to find a solution.

[Ahajha]

I might have a solution that seems to make the compiler happy, using a bit of template metaprogramming (everyone's favorite (: )

      template <class, class>
      struct member_check;

      template <class T, class mptr_t>
      requires std::is_member_pointer_v<std::decay_t<mptr_t>>
      struct member_check<T, mptr_t>
      {
         using type = decltype(std::declval<T>().*std::decay_t<mptr_t>{});
      };

      template <class T, class mptr_t>
      requires(!std::is_member_pointer_v<std::decay_t<mptr_t>> &&
               std::is_enum_v<std::decay_t<T>>)
      struct member_check<T, mptr_t>
      {
         using type = T;
      };

      template <class T, class mptr_t>
      requires(!std::is_member_pointer_v<std::decay_t<mptr_t>> &&
               !std::is_enum_v<std::decay_t<T>>)
      struct member_check<T, mptr_t>
      {
         using type = std::invoke_result_t<mptr_t, T>;
      };

Then we can replace

std::decay_t<decltype(member_check<T, std::tuple_element_t<
             1, std::tuple_element_t<I, meta_t<T>>>>())>...>{};

with

std::decay_t<member_check<T, std::tuple_element_t<
             1, std::tuple_element_t<I, meta_t<T>>>>()>...>{};

This still doesn't build, but I think the other failures are due to unrelated problems.

I might open a PR if I can fix all the other issues.

[stephenberry]

Nice, this is a great idea. Your use of member_check was almost there. It just needed to be:

 std::decay_t<typename member_check<T, std::tuple_element_t<
                              1, std::tuple_element_t<I, meta_t<T>>>>::type>...>{};

The latest commit has this change, so see if gcc builds now for you. Apparently Xcode 14 breaks things with gcc linking, so I'm currently able to build but not link on my machine.
3cdddb9

[Ahajha]

Ah, the typename issue, if I recall it shouldn't be necessary in C++20 but clang doesn't support that yet (coming in clang 16!) so that's why I didn't catch it.

There are two more issues I'm running into, the first is with code like the following in json/study.hpp:

result.gen() =
                  [this, dist = std::normal_distribution<double>(
                                        mean, std_dev)]() {
                  return dist(this->engine);
               };

The compiler complains about no matching function call, yada yada. The fix is to make the lambdas mutable:

result.gen() =
                  [this, dist = std::normal_distribution<double>(
                                        mean, std_dev)]() mutable {
                  return dist(this->engine);
               };

(There are 4 of these in that file)

The second issue I haven't figured out yet is this:

[build] /home/alex/Documents/glaze/tests/json_test/json_test.cpp:644:15:   required from here
[build] /home/alex/Documents/glaze/include/glaze/core/common.hpp:357:10: error: invalid cast from type ‘const char*’ to type ‘Color’
[build]   357 |          To{from};
[build]       |          ^~~~~~~~

[stephenberry]

Pushed a fix for mutable lambdas (72151c2). I'm actually surprised other compilers didn't complain about this.

[stephenberry]

That To{from}; error is strange, because it is coming from a concept, which is supposed to fail in that case.

[Ahajha]

Yea, and as far as I can tell it doesn't interact with "Color" at all.

I think I have something, but I'm not 100% sure if it's what you want:

      template <class From, class To>
      concept non_narrowing_convertable = requires(From from, To to)
      {
         to = from;
      };

The reason I changed it to this is because when you use it, you check the concept, and then immediately do the equivalent of to = from:

            if constexpr (std::is_assignable_v<decltype(val),
                                               decltype(value)> &&
                          detail::non_narrowing_convertable<
                             std::decay_t<decltype(value)>,
                             std::decay_t<decltype(val)>>) {
               result = true;
               val = value;
            }

There are many compiler warnings produced at this point, but it does at least compile now.

[stephenberry]

That change checks if it is assignable, which should compile fine, but isn't as safe as blocking narrowing conversions. But, I'd rather get gcc compiling, so I'm changing the code to:

      template <class From, class To>
      concept non_narrowing_convertable = requires(From from, To to)
      {
#if __GNUC__
         // TODO: guard gcc against narrowing conversions when fixed
         to = from;
#else
         To{from};
#endif
      };

Yeah, I'm slowly working through warnings. A bunch come from the fmt library. But, I'm really glad gcc is building now! Thanks!

[stephenberry]

Closing this issue. Will kick off another issue to add automatic build/unit testing on pushes.