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

Clang complains about concept depending on itself #62096

Open
craffael opened this issue Apr 12, 2023 · 21 comments
Open

Clang complains about concept depending on itself #62096

craffael opened this issue Apr 12, 2023 · 21 comments
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts

Comments

@craffael
Copy link

craffael commented Apr 12, 2023

Version of Clang

clang version 17.0.0 (https://github.com/llvm/llvm-project.git 5b461d5ec172d21029da492064704fe3da6f8bab)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /opt/compiler-explorer/clang-trunk/bin

Steps to reproduce the problem

Try to compile the following program with clang++ main.cpp -std=c++20 -ftemplate-backtrace-limit=0:

#include <type_traits>

template <class OPERATOR>
concept Operator = std::is_nothrow_copy_constructible_v<OPERATOR>;

struct AnyOperator {

  template <Operator OP>
  explicit AnyOperator(OP op) noexcept {}

  /// Copy constructor
  AnyOperator(const AnyOperator&) noexcept = default;
};


static_assert(Operator<AnyOperator>);

It produces the following output (see also Godbolt):

<source>:4:20: error: satisfaction of constraint 'std::is_nothrow_copy_constructible_v<OPERATOR>' depends on itself
concept Operator = std::is_nothrow_copy_constructible_v<OPERATOR>;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:4:20: note: while substituting template arguments into constraint expression here
concept Operator = std::is_nothrow_copy_constructible_v<OPERATOR>;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:8:13: note: while checking the satisfaction of concept 'Operator<AnyOperator>' requested here
  template <Operator OP>
            ^
<source>:8:13: note: while substituting template arguments into constraint expression here
  template <Operator OP>
            ^~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.1/../../../../include/c++/13.0.1/type_traits:3306:7: note: while checking constraint satisfaction for template 'AnyOperator<AnyOperator>' required here
    = __is_nothrow_constructible(_Tp, __add_lval_ref_t<const _Tp>);
      ^~~~~~~~~~~~~~~~~~~~~~~~~~
/opt/compiler-explorer/gcc-snapshot/lib/gcc/x86_64-linux-gnu/13.0.1/../../../../include/c++/13.0.1/type_traits:3306:7: note: in instantiation of function template specialization 'AnyOperator::AnyOperator<AnyOperator>' requested here
<source>:4:25: note: in instantiation of variable template specialization 'std::is_nothrow_copy_constructible_v<AnyOperator>' requested here
concept Operator = std::is_nothrow_copy_constructible_v<OPERATOR>;
                        ^
<source>:4:20: note: while substituting template arguments into constraint expression here
concept Operator = std::is_nothrow_copy_constructible_v<OPERATOR>;
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<source>:16:15: note: while checking the satisfaction of concept 'Operator<AnyOperator>' requested here
static_assert(Operator<AnyOperator>);
              ^~~~~~~~~~~~~~~~~~~~~
<source>:16:1: error: static assertion failed
static_assert(Operator<AnyOperator>);
^             ~~~~~~~~~~~~~~~~~~~~~
<source>:16:15: note: because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression
static_assert(Operator<AnyOperator>);
              ^
2 errors generated.

Correct behavior

Clang should accept the code. As far as I know the code is valid. It compiles fine with MSVC and GCC.

@EugeneZelenko EugeneZelenko added c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts and removed new issue labels Apr 12, 2023
@llvmbot
Copy link
Member

llvmbot commented Apr 12, 2023

@llvm/issue-subscribers-clang-frontend

@llvmbot
Copy link
Member

llvmbot commented Apr 12, 2023

@llvm/issue-subscribers-c-20

@shafik
Copy link
Collaborator

shafik commented Apr 12, 2023

I think this should work but really @erichkeane should take a look

@erichkeane
Copy link
Collaborator

I don't see any reason this shouldn't work, that looks right? And I admit the error message doesn't do a great job telling us what went wrong unfortunately. However, can anyone create a min-repro for this for me that doesn't include the include of the type trait? I wonder if the type trait is doing something goofy here.

@erichkeane
Copy link
Collaborator

erichkeane commented Apr 12, 2023

Actually, I got it down to:

template <class OPERATOR>
concept Operator =
  __is_nothrow_constructible(OPERATOR, OPERATOR&);

struct AnyOperator {

  template <Operator OP>
  explicit AnyOperator(OP op) noexcept {}

  AnyOperator(const AnyOperator&) noexcept = default;
};

static_assert(Operator<AnyOperator>);

Unless that builtin has something wacky happening, I don't see the recursion. The code that does just checks to see if we're still in the evaluation of something while being asked to evaluate the same constraint, but I'm not sure where that could have come from.

@craffael
Copy link
Author

I can only guess: If your remove the templated constructor it works. So I assume that:

  1. We check whether AnyOperator fulfills the Operator concept.
  2. I.e. we have to check whether AnyOperator is copy constructible. But for some reason not the copy constructor is selected, but the templated constructor: AnyOperator::AnyOperator<AnyOperator>
  3. Now the compiler checks again whether AnyOperator fulfills the Operator concept. Now we are back at step 1 and we have a recursion...

@erichkeane
Copy link
Collaborator

AAh, so that builtin DOES cause this problem, and this is a legitimate diagnostic as far as I can tell. If I remove the diagnostic, we hit our template implementation limit.

__is_nothrow_constructible is causing us to evaluate whether AnyOperator is constructible from an AnyOperator, which causes it to have to re-evaluate Operator with AnyOperator (for the constraint on AnyOperator::AnyOperator). I'm not sure how GCC is managing this, unless __is_nothrow_constructible has a way of constructing it without evaluating the constraint on the constructor.

So at the moment, I don't think this is a problem with the concepts stuff, but with that evaluation of the builtin. SO I'm not sure what I can do about it. @shafik : Perhaps its worth looking into that builtin/how GCC behaves with it?

@erichkeane
Copy link
Collaborator

I can only guess: If your remove the templated constructor it works. So I assume that:

1. We check whether AnyOperator fulfills the Operator concept.

2. I.e. we have to check whether AnyOperator is copy constructible. But for some reason not the copy constructor is selected, but the templated constructor: `AnyOperator::AnyOperator<AnyOperator>`

3. Now the compiler checks again whether `AnyOperator` fulfills the Operator concept. Now we are back at step 1 and we have a recursion...

Yep! Looks like we simul-posted, thats exactly what is happening. The builtin is effectively doing OPERATOR(OPERATOR&);, which is re-evaluating the constraint on the ctor. So I'm not sure what we can do about that. I still don't know how GCC figures that out, even using the same builtin.

@craffael
Copy link
Author

craffael commented Apr 12, 2023

I think the problem is that __is_nothrow_constructible selects the wrong overload. If I write

AnyOperator o = ...; // somehow initialize it
AnyOperator q(o);

then the copy constructor should be selected (and not the template constructor). GCC and Clang (for Clang we must remove the concept constraint) both select the copy constructor, i.e. they are both correct. But __is_nothrow_constructible somehow has problems, respectively seems to use some other logic to make the decision...

@erichkeane
Copy link
Collaborator

Hmm... yes, I think thats it. We've somehow decided that our initialization here requires evaluating the template, even though there is already a non-template match. But I don't know our initialization code well enough to know the answer to that. Perhaps there is a missing DR here that we aren't implementing @hubert-reinterpretcast ?

@JVApen
Copy link

JVApen commented Apr 13, 2023

Reading this, it reminds me of the following issues: #40268 and #38149
There is something specific about the overload resolving that does not choose the copy constructor if not called with a const instance.

@tigrkoshka

This comment was marked as duplicate.

@erichkeane

This comment was marked as resolved.

@ckwastra

This comment was marked as off-topic.

@hubert-reinterpretcast
Copy link
Collaborator

Perhaps there is a missing DR here that we aren't implementing @hubert-reinterpretcast ?

Clang is doing the "directly conforming thing". The template is a candidate (https://wg21.link/over.match.ctor), and deduction (including checking of constraints; https://wg21.link/temp.deduct.general#5) happens before the rest of overload resolution. The only de jure reason that the copy constructor is the better candidate is that it is not a template (the template results in a viable candidate).

GCC seems to "figure it out" by applying a special (ad hoc) rule against constructors that take their own class by value as the sole argument. It never bothered to check the constraints.

https://godbolt.org/z/4nqETK5ba

template <typename T>
struct Oops {
  static_assert(sizeof(T) == 0);  // GCC never sees this
  static constexpr bool value = true;
};

template <class OPERATOR>
concept Operator = Oops<OPERATOR>::value;

struct AnyOperator {
  template <Operator OP>
  AnyOperator(OP op) noexcept {}

  explicit AnyOperator(const AnyOperator&) noexcept = default;
};

void f(AnyOperator *ap) {
  AnyOperator a = *ap;
}

@hubert-reinterpretcast
Copy link
Collaborator

hubert-reinterpretcast commented Jun 12, 2024

I don't see a defect here: The original reporter should simply have added a helper concept that checks that the constructor template argument is not AnyOperator prior to checking the constructibility.

@hubert-reinterpretcast
Copy link
Collaborator

hubert-reinterpretcast commented Jun 12, 2024

The closest "rule" appears to be https://wg21.link/class.copy.ctor#5.

However, the words:

A member function template is never instantiated to produce such a constructor signature.

technically mean that the definition is not produced (not the declaration, despite references to "signature").

There is also nothing in those words that indicate where in the deduction/substitution/constraint checking/etc. process this kicks in.

@hubert-reinterpretcast
Copy link
Collaborator

It appears that Clang also has some version of the "rule".

https://godbolt.org/z/hT4nfvxnW

struct AnyOperator {
  template <typename OP>
  AnyOperator(OP op, int = 0) noexcept {}  // Clang rejects this candidate for the copy initialization of `op`

  explicit AnyOperator(const AnyOperator&) noexcept {}
};

void f(AnyOperator *ap) {
  AnyOperator a(*ap, 0);
}

@hubert-reinterpretcast
Copy link
Collaborator

I encountered a similar issue regarding operator overloading where MSVC/GCC accept the code (Godbolt):

template <class T, class U, class = void> struct A : false_type {};
template <class T, class U>
struct A<T, U, void_t<decltype(declval<T>() + declval<U>())>> : true_type {};
template <class T, class U> void operator+(T, U);
template <class T, class U>
  requires A<T, U>::value
  // ^ Satisfaction of constraint 'A<T, U>::value' depends on itself
void operator+(T, U);
int main() {
  auto a = {1, 2, 3};
  auto b = {1, 2, 3};
  a + b; // Invalid operands to binary expression
}

Using std::enable_if_t instead of requires seems to resolve this issue.

Yours is a different issue @ckwastra. The originally-reported issue hinges on when a constructor template candidate should be rejected. Your issue is a bug in Clang's name lookup for dependent operator function calls using the overloaded operator syntax.

namespace Q {
  struct A {};
}

template <class T>
int operator+(T, char (*)[0 ?
  T{} + static_cast<char (*)[1]>(0) :
  1]);  // Not recursive: `operator+` only available to unqualified name lookup following the end of the declarators

int f() {
  return Q::A{} + static_cast<char (*)[1]>(0);
}

https://godbolt.org/z/K3zxGPn1z

@hubert-reinterpretcast
Copy link
Collaborator

However, the words:

A member function template is never instantiated to produce such a constructor signature.

technically mean that the definition is not produced (not the declaration, despite references to "signature").

There is also nothing in those words that indicate where in the deduction/substitution/constraint checking/etc. process this kicks in.

cplusplus/CWG#474

@zygoloid
Copy link
Collaborator

In addition to the question of when the "substitution produces a C(C) constructor" check takes place, there's another GCC / Clang difference here: GCC appears to not consider template candidates at all if there's an exact match among the non-template candidates:

template <typename T>
struct Oops {
  static_assert(sizeof(T) == 0);
  static constexpr bool value = true;
};

template <class OPERATOR>
concept Operator = Oops<OPERATOR>::value;

template <Operator OP> void f(OP op);
void f(int);

// Clang rejects, GCC accepts
void g(int n) { f(n); }
// Both reject
void h(short n) { f(n); }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" concepts C++20 concepts
Projects
Status: No status
Development

No branches or pull requests

10 participants