llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
26.71k stars 10.94k forks source link

Cannot use lambda expression within Requires #96408

Open Fedr opened 1 week ago

Fedr commented 1 week ago

This program

template<typename T>
concept C = requires(T t) {
    [x=t](){ return x; }();
};

static_assert(C<int>);

seems valid, and it is accepted by GCC and MSVC. Unfortunately, Clang rejects it with the

error: constraint variable 't' cannot be used in an evaluated context
    3 |     [x=t](){ return x; }();

Online demo: https://gcc.godbolt.org/z/37v5bWKvv

llvmbot commented 1 week ago

@llvm/issue-subscribers-clang-frontend

Author: Fedor Chelnokov (Fedr)

This program ``` template<typename T> concept C = requires(T t) { [x=t](){ return x; }(); }; static_assert(C<int>); ``` seems valid, and it is accepted by GCC and MSVC. Unfortunately, Clang rejects it with the ``` error: constraint variable 't' cannot be used in an evaluated context 3 | [x=t](){ return x; }(); ``` Online demo: https://gcc.godbolt.org/z/37v5bWKvv
llvmbot commented 1 week ago

@llvm/issue-subscribers-c-20

Author: Fedor Chelnokov (Fedr)

This program ``` template<typename T> concept C = requires(T t) { [x=t](){ return x; }(); }; static_assert(C<int>); ``` seems valid, and it is accepted by GCC and MSVC. Unfortunately, Clang rejects it with the ``` error: constraint variable 't' cannot be used in an evaluated context 3 | [x=t](){ return x; }(); ``` Online demo: https://gcc.godbolt.org/z/37v5bWKvv
zyn0217 commented 1 week ago

This is likely cwg2517.

hubert-reinterpretcast commented 1 week ago

This is likely cwg2517.

CWG 2517 does not apply here because the case presented here uses a simple-requirement (which does not introduce a context requiring evaluation). According to https://wg21.link/expr.prim.req.general#2:

Expressions appearing within a requirement-body are unevaluated operands

zyn0217 commented 1 week ago

This is likely cwg2517.

CWG 2517 does not apply here because the case presented here uses a simple-requirement (which does not introduce a context requiring evaluation). According to https://wg21.link/expr.prim.req.general#2:

Expressions appearing within a requirement-body are unevaluated operands

Hmmm.. Yeah, however, we (the implementation of checking whether the use of a local parameter is within an unevaluated context) don't tell apart the nested-requirement and simple-requirement currently. And I would say this can be resolved if we implement cwg2517.

zyn0217 commented 1 week ago

To clarify how we end up with this diagnostic:

  1. Parse the requires expression in ParseRequiresExpression() with an Unevaluated context having been pushed.
  2. Recurse into the simple-requirement i.e. the lambda in the example, in ParseExpression().
  3. Push a PotentiallyEvaluated evaluation context in ParseLambdaIntroducer().
  4. In DiagnoseUseOfDecl(), we only check the last evaluation context (PotentiallyEvaluated now) and if the Decl to which we are referencing appears within the parameter list of a requires expression. If so, diagnose it as invalid.

So the solution would be either:

a. Refine the logic in ParseLambdaIntroducer() to not push a PotentiallyEvaluated context if we're currently inside a requires expression. b. Remove the diagnostic in 4), which is incorrect regarding lambdas in requirements and is useless regarding CWG2517.

However, I would prefer doing both because we might have other reliances to evaluation kinds elsewhere...

hubert-reinterpretcast commented 1 week ago

a. Refine the logic in ParseLambdaIntroducer() to not push a PotentiallyEvaluated context if we're currently inside a requires expression.

I'm not sure a requires expression is special here. Why do we push a PotentiallyEvaluated context there?

GCC, MSVC, and EDG all accept this (Clang doesn't):

struct A {
  A() = default;
  A(const A &) = delete;
};
bool f() {
  A a;
  return [=] { return sizeof([ap = &a] {}); }();
}
hubert-reinterpretcast commented 1 week ago

GCC, MSVC, and EDG all accept this (Clang doesn't)

https://wg21.link/basic.def.odr#3 and https://wg21.link/intro.execution#3 through 4 tells us that Clang is wrong.

zyn0217 commented 1 week ago

Why do we push a PotentiallyEvaluated context there?

Hmmm, this dates back to a ~10-year-ago patch, and I need some time to investigate why.