Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-bool, non-constant expressions in requires clauses: SFINAE or hard error? #45

Closed
Quuxplusone opened this issue Jan 17, 2019 · 6 comments
Labels
bug Something isn't working duplicate This issue or pull request already exists

Comments

@Quuxplusone
Copy link

https://concepts.godbolt.org/z/MTt3Hc

constexpr int x = 42;

template<class T>
void f() requires T(x) {}

template<class T>
void f() {}

int main()
{
    f<int>();
    f<bool&>();
}

Clang claims f<int>() is unambiguous (the constrained f doesn't participate in overload resolution because int(x) is not boolish).
Clang claims f<bool&>() is a hard error (the constrained f produces a hard error because (bool&)(x) is not a constant-expression).

FWIW, here's GCC's behavior:
GCC claims f<int>() is a hard error (the constrained f produces a hard error because int(x) is not boolish).
GCC claims f<bool&>() is a hard error (the constrained f produces a hard error because (bool&)(x) is not a constant-expression).

I suspect that the correct behavior would be to treat both f<int>() and f<bool&>() as unambiguous.

@saarraz
Copy link
Owner

saarraz commented Jan 29, 2019

Actually, what is happening here is related to a deeper problem - the trailing requires clause of the first f is instantiated as part of the template instantiation, which causes a SFINAE error, which is why that f is not participating.
If you put the requires clause in the template-head instead, you get behavior that's consistent with GCC: https://concepts.godbolt.org/z/T9qYOr

http://eel.is/c++draft/temp.constr.constr#temp.constr.atomic-3 leads me to think GCC's right here.
Anyway this issue is caused by #32

@saarraz saarraz added bug Something isn't working duplicate This issue or pull request already exists labels Jan 29, 2019
@Quuxplusone
Copy link
Author

Oh, gross. http://eel.is/c++draft/temp.constr.constr#temp.constr.atomic-3 currently says:

To determine if an atomic constraint is satisfied, the parameter mapping and template arguments are first substituted into its expression. If substitution results in an invalid type or expression, the constraint is not satisfied. Otherwise, the lvalue-to-rvalue conversion is performed if necessary, and E shall be a constant expression of type bool. The constraint is satisfied if and only if evaluation of E results in true.

So:

  • If E is "invalid" (ill-formed?), then the constraint is unsatisfied.
  • If E is valid but not a constant-expression, then we have undefined behavior.
  • If E is valid and constant-expression, but not of type bool, then we have undefined behavior.
  • If E is valid, constant-expression, and bool, but false, then the constraint is unsatisfied.
  • If E is valid, constant-expression, bool, andtrue, then the constraint is satisfied.

This makes requires-clauses strictly more "UB-ish" than enable_if (which, being a simple library facility, never has undefined behavior). Worse, this UB is completely gratuitous, since by definition the compiler can detect it.

@saarraz, I strongly believe that Clang should implementation-define the undefined behavior here. In the cases where the paper standard leaves the language's behavior undefined, Clang should act as if the constraint was unsatisfied (and thus the overload in question should SFINAE away).

@saarraz
Copy link
Owner

saarraz commented Jan 29, 2019

Isn't this behavior a bit confusing to people who don't know the rules? Someone can use an int as a condition and expect it to act like it would in an if, being true if it's not 0, but it would never be satisfied

@CaseyCarter
Copy link
Contributor

CaseyCarter commented Jan 29, 2019

"shall" is a diagnosable constraint in Core wording: non-constant expressions and expressions of non-bool type don't produce undefined behavior, they result in the program being ill-formed.

I agree that it's insane that these corner cases produce an ill-formed program rather than resulting in the dissatisfaction of the concept, but Core is more concerned about the potential for writing buggy concepts that are never satisfied than the fact that these rules are extremely novice-hostile. (See http://lists.isocpp.org/ext/2018/08/5383.php which requires reflector access.)

@Quuxplusone
Copy link
Author

@CaseyCarter, your message in http://lists.isocpp.org/ext/2018/08/5383.php sounds accurate to me. It is crazy that the overload set https://godbolt.org/z/Snbyrm

template<class T> void foo(T t) requires (T::is_transparent) { puts("YES"); }
template<class T> void foo(T t) { puts("NO"); }

should give hard errors on types like struct T { static constexpr int is_transparent = true; }, and yet be a perfectly valid overload set for types like struct less { using is_transparent = void; }. If requires-clauses aren't even going to SFINAE away properly, then I don't see any benefit to using requires-clauses over the existing ways we have to do SFINAE today. It seems like it's just making the user's code more fragile.

@saarraz
Copy link
Owner

saarraz commented Feb 9, 2019

Fixed in 05d5da4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants