[concepts] deferred substitution into requirements of class template members not implemented

[marehr]

Bugzilla Link	44833
Version	trunk
OS	Linux
CC	@CaseyCarter,@erichkeane,@friedkeenan,@h-2,@JohelEGP,@AlexeyDmitriev,@PiliLatiesa,@zygoloid,@saarraz,@HighCommander4,@jwakely

Extended Description

Sorry for this verbose and hard to read code, but I wanted to make this configurable.

This code basically has multiple code paths. If REQUIRES_BUG is set, the impl function uses requires to infer if char_is_valid_for is defined or not. If not impl uses SFINAE.

#include <utility>

// #define REQUIRES_BUG     1 - enable the requires bug, 0 - disable it
// #define FUNCTION_DEFINED 1 - define function char_is_valid_for, 0 - do not define char_is_valid_for 

template <int I> struct priority_tag : priority_tag<I - 1> {};
template <> struct priority_tag<0> {};

struct alphabet_base {};

#if FUNCTION_DEFINED
using expect_return_type = bool;
bool char_is_valid_for(char, alphabet_base) { return true; };
#else
using expect_return_type = int;
#endif

namespace seqan3::detail::adl_only {

template <typename...> void char_is_valid_for(...) = delete;

template <typename alph_t>
struct char_is_valid_for_fn
{
    using s_alph_t = alph_t;

#if REQUIRES_BUG
    template <typename t>
    static decltype(auto) impl(priority_tag<1>, t v)
        requires requires { (char_is_valid_for(v, s_alph_t{})); }
    { return (char_is_valid_for(v, s_alph_t{})); }
#else
    template <typename t>
    static auto impl(priority_tag<1>, t v)
        -> std::enable_if_t<true, decltype(char_is_valid_for(v, s_alph_t{}))>
    { return (char_is_valid_for(v, s_alph_t{})); }
#endif

    template <typename t, typename = void>
    static decltype(auto) impl(priority_tag<0>, t v) { return 0; }
};
} // namespace seqan3::detail::adl_only

int main() {
  auto a = seqan3::detail::adl_only::char_is_valid_for_fn<alphabet_base>::impl(priority_tag<1>{}, char{});
  static_assert(std::is_same_v<decltype(a), expect_return_type>);
}

As you can see on https://godbolt.org/z/tyeiJ2 when defining clang++ -std=c++2a -DREQUIRES_BUG=1 -DFUNCTION_DEFINED=0 the requires block should silently see that the requires is not valid (because of a deleted function) and skip to the next definition.

Error Message:

<source>:31:15: error: call to deleted function 'char_is_valid_for'
    { return (char_is_valid_for(v, s_alph_t{})); }
              ^~~~~~~~~~~~~~~~~

<source>:45:75: note: in instantiation of function template specialization 'seqan3::detail::adl_only::char_is_valid_for_fn<alphabet_base>::impl<char>' requested here

  auto a = seqan3::detail::adl_only::char_is_valid_for_fn<alphabet_base>::impl(priority_tag<1>{}, char{});
                                                                          ^

<source>:20:29: note: candidate function [with $0 = <>] has been explicitly deleted
template <typename...> void char_is_valid_for(...) = delete;
                            ^

<source>:45:8: error: variable has incomplete type 'void'
  auto a = seqan3::detail::adl_only::char_is_valid_for_fn<alphabet_base>::impl(priority_tag<1>{}, char{});
       ^

2 errors generated.

Interestingly, if you substitute s_alph_t with alph_t; everything works as expected.

[zygoloid]

The difference between GCC and Clang is due to an open question for C++20 concepts: do we defer substitution into member requires-clauses or not? The current thinking in the committee is that we do not, which is what Clang implements (the older direction was that we would defer such substitution, which is what GCC implements).

You can avoid this problem by using a named concept instead of an inline requires-clause: https://godbolt.org/z/XCjyw_

[zygoloid]

(Using a named concept allows the code to be written a bit more concisely too: https://godbolt.org/z/QsSnXM)

[marehr]

Thank you Richard Smith for the quick response.

So if I understand you correctly an inline requires is currently okay, if I don't use anything declared within the context of the class.

That means

• https://godbolt.org/z/B_hMwk is "undefined behaviour" because I declare s_alph_t within the class and use it in an inline requires
• moving the typedef using s_alph_t = alph_t; into the template parameter is okay, because it is outside of the class (https://godbolt.org/z/yffhHc)
• or using named concepts which are per definition not inside the class (https://godbolt.org/z/aJx4C9)

Do you know what the voting in the last meeting on this was and what the resolution is?

As a user I think of inline requires (if you don't use them in combination with named concepts) as glorified enable_if's with better diagnostic output. Because subsumption can only take place with named concepts, so I personally would prefer if they behave the same.

This code was reduced and was originally part of a macro that made CPO overloads easier, if I have to use named concepts this would defeat the purpose of that macro.

#define SEQAN3_CPO_IMPL(PRIO, TERM) \
template <typename t, typename ...arg_ts> \
static constexpr decltype(auto) impl(seqan3::detail::priority_tag<PRIO> const &, t && v, [[maybe_unused]] arg_ts && ... args) \
    requires requires (seqan3::detail::priority_tag<PRIO> const &, t && v, arg_ts && ... args) \
    { { TERM }; } \
{ return TERM; }

But maybe we have to find a different solution.

On a side note, none of my/our solutions work with msvc.

[zygoloid]

Do you know what the voting in the last meeting on this was and what the resolution is?

The rule that we decided on is this:

• Substitution of template arguments into a class does not substitute into constraints, ever.
• Constrained friends in classes that mention template arguments from outside the friend are "unique", do not redeclare anything else, and must be definitions. (This avoids the need to substitute into their constraints for mangling and declaration matching.)
• Declaration matching for out-of-class explicit specializations, instantiations, and so on, of a member of a class template does perform full substitution into the constraints; substitution failure in the immediate context results in the declaration not matching.

This is closer to GCC's behavior than to Clang's, and in particular means your example is valid and this is a Clang bug.

[CaseyCarter]

+1, stumbled across this implementing std::ranges::view_interface (which is specified to expect "Thou shalt not substitute into requires-clauses before overload resolution!").

[llvmbot]

Is this the same issue that prevents the use of ranges from GCC 10.2.0 libstdc++?

https://godbolt.org/z/f51ses

Which boils down to:

template <typename D>
struct CRTP {
   void call_foo() requires
     requires (D& v) { v.foo();} {
       static_cast<D*>(this)->foo();
   }
};

struct Test: public CRTP<Test> {
   void foo() { }
};

int main()
{
   Test t;
   t.call_foo();
   return 0;
}

<source>:16:7: error: invalid reference to function 'call_foo': constraints not satisfied
   t.call_foo();
     ^
<source>:4:27: note: because 'v.foo()' would be invalid: no member named 'foo' in 'Test'
     requires (D& v) { v.foo();} {
                         ^

https://godbolt.org/z/8jrz9q

[zygoloid]

*** Bug llvm/llvm-bugzilla-archive#47508 has been marked as a duplicate of this bug. ***

[PiliLatiesa]

+1

Having to use a named concept or boolean variable is counter-intuitive.

[friedkeenan]

I believe this is a genuine bug, as the standard seems to expect that the requires clause of a method is only evaluated when the method is used, see std::ranges::view_interface: https://eel.is/c++draft/view.interface

Those requires clauses can't be satisfied with incomplete types.

[erichkeane]

Is this the same issue that prevents the use of ranges from GCC 10.2.0
libstdc++?

https://godbolt.org/z/f51ses

Which boils down to:

template <typename D>
struct CRTP {
   void call_foo() requires
     requires (D& v) { v.foo();} {
       static_cast<D*>(this)->foo();
   }
};

struct Test: public CRTP<Test> {
   void foo() { }
};

int main()
{
   Test t;
   t.call_foo();
   return 0;
}

<source>:16:7: error: invalid reference to function 'call_foo': constraints
not satisfied
   t.call_foo();
     ^
<source>:4:27: note: because 'v.foo()' would be invalid: no member named
'foo' in 'Test'
     requires (D& v) { v.foo();} {
                         ^

https://godbolt.org/z/8jrz9q

Thanks for the reproducer! I've poked around this for a few days, and this example is pretty clarifying. In this case, the constraint fails because we've been evaluating it immediately upon instantiation of the class (in the definition of 'Test''s inheritence), rather than waiting until it is called.

I see that Richard posted about it here:

Substitution of template arguments into a class does not substitute into constraints, ever.

Which I think is likely the solution to the ranges problem as well! Unfortunately, it is a pretty significant architectural change to make this happen as far as I can tell. I'm still looking into it, as compiling ranges is pretty important for us.

[erichkeane]

Do you know what the voting in the last meeting on this was and what the resolution is?

The rule that we decided on is this:

• Substitution of template arguments into a class does not substitute
  into constraints, ever.
• Constrained friends in classes that mention template arguments from
  outside the friend are "unique", do not redeclare anything else, and must be
  definitions. (This avoids the need to substitute into their constraints for
  mangling and declaration matching.)
• Declaration matching for out-of-class explicit specializations,
  instantiations, and so on, of a member of a class template does perform
  full substitution into the constraints; substitution failure in the
  immediate context results in the declaration not matching.

This is closer to GCC's behavior than to Clang's, and in particular means
your example is valid and this is a Clang bug.

Hi Richard-
I'm trying to work through this(at least the 1st bullet at least), but am having trouble finding the appropriate place to do the instantiation work for a called function. I was wondering if you had a link to the wording that made that change? Or some suggestion on how to go about doing those checks?

[jwakely]

+1, stumbled across this implementing std::ranges::view_interface (which is
specified to expect "Thou shalt not substitute into requires-clauses before
overload resolution!").

Is this the same issue that prevents the use of ranges from GCC 10.2.0
libstdc++?

Yes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102807

[AaronBallman]

*** #49634 has been marked as a duplicate of this bug. ***

[h-2]

Is there any progress on this issue? Does it indeed break all usage of views from libstdc++?

[zygoloid]

mentioned in #46852