llvm / llvm-project

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

[Clang] consteval functions called in NSDMI during aggregate initialization rejected if they use `this` #104908

Open MitalAshok opened 3 weeks ago

MitalAshok commented 3 weeks ago

Similar to https://eel.is/c++draft/expr.const#example-9

https://godbolt.org/z/zY3x1foG7

consteval int id(int i) { return i; }

struct A {
  int x;
  int y = id(x);
} a{42};

Clang rejects this:

<source>:5:11: error: call to consteval function 'id' is not a constant expression
    5 |   int y = id(x);
      |           ^
<source>:6:7: note: in the default initializer of 'y'
    6 | } a{42};
      |       ^
<source>:5:3: note: declared here
    5 |   int y = id(x);
      |   ^
<source>:5:14: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
    5 |   int y = id(x);
      |              ^

MSVC also rejects this but GCC does not.

Clang does accept a{ 42, id(a.x) }

MitalAshok commented 3 weeks ago

I see that example is here: https://github.com/llvm/llvm-project/blob/6dcce422ca06601f2b00e85cc18c745ede245ca6/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp#L55-L58

The comment in the example says "k<int> is not an immediate function because A(42) is a constant expression and thus not immediate-escalating", yet int(*p)(int) = &k<int>; fails claiming it's an immediate function: https://godbolt.org/z/9e9eqhzEW

llvmbot commented 3 weeks ago

@llvm/issue-subscribers-clang-frontend

Author: Mital Ashok (MitalAshok)

Similar to https://eel.is/c++draft/expr.const#example-9 https://godbolt.org/z/zY3x1foG7 ```c++ consteval int id(int i) { return i; } struct A { int x; int y = id(x); } a{42}; ``` Clang rejects this: ``` <source>:5:11: error: call to consteval function 'id' is not a constant expression 5 | int y = id(x); | ^ <source>:6:7: note: in the default initializer of 'y' 6 | } a{42}; | ^ <source>:5:3: note: declared here 5 | int y = id(x); | ^ <source>:5:14: note: implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function 5 | int y = id(x); | ^ ``` MSVC also rejects this but GCC does not. Clang *does* accept `a{ 42, id(a.x) }`
cor3ntin commented 3 weeks ago

duplicate of #63139

Hum, maybe not! Might just be a case of a missing ThisScope in BuildCXXDefaultInitExpr

MitalAshok commented 3 weeks ago

This example was added with P2564R3.

Using the NSDMI with a constructor does work:

https://godbolt.org/z/3eYoMax89

consteval int id(int i) { return i; }

struct A {
  int x = 42;
  int y = id(x);
} a;

// No error calling the implicitly declared immediate default constructor
// (It's immediate because it's constexpr and the initializer for y is immediate-escalating: CWG2760)

Also the error in the other case:

implicit use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function

makes it seem like aggregate initialization (which does not call any member function) wasn't considered properly

zygoloid commented 3 weeks ago

Isn't clang correct here? The immediate invocation is just id(x), not the entire initialization of a, so it can't read from a by http://eel.is/c++draft/expr.const#5.9.2 because its lifetime didn't start in the evaluation of E.

MitalAshok commented 3 weeks ago

http://eel.is/c++draft/expr.const#16.sentence-3:

An aggregate initialization is an immediate invocation if it evaluates a default member initializer that has a subexpression that is an immediate-escalating expression.

So the whole initializer should be an immediate invocation.

Even without that, [expr.const]p(20.5) should have made the initializer for a manifestly constant evaluated (an immediate function context per [expr.const]p(16.2)), meaning the id(x) should not have been an immediate invocation since it is a subexpression of a constant expression:

An expression or conversion is manifestly constant-evaluated if it is:

  • the initializer of a variable that is usable in constant expressions or has constant initialization.

So:

consteval int id(int i) { return i; }

struct A {
  int x;
  int y = id(x);
} a{42};  // There is not immediate invocation or immediate-escalating expression, just a manifestly constant expression

int main() {
  return A{42}.y;  // `A{42}` is an immediate invocation
}

It appears that the first issue was fixed between clang 18 and main: https://godbolt.org/z/Y1sr3Kv5W

#include <vector>

consteval std::vector<int> f() {
    return { 1, 2, 3 };
};

/*constinit*/ int i = f().size();
// f() is not an immediate invocation, the entire initialization is manifestly constant evaluated.
// Should not complain about the memory not being deallocated since it *is* when the temporary is destroyed

But I can't find the issue/pr that fixed this, and it doesn't seem to apply to NSDMIs used in aggregate initialization

cor3ntin commented 3 weeks ago

In BuildCXXDefaultInitExpr, we should set ExprEvalContexts.back().InImmediateEscalatingFunctionContext = true;

But more critically, all of your handling of "immediate invocations" is based on function, so this is going to be tricky to solve... In the case of an aggregate initializer we don't have a good place to track the immediateness and attach the whole machinery to.

shafik commented 3 weeks ago

Note gcc is the only one out of clang/edg/MSVC to accept this: https://godbolt.org/z/56Yoa8v5z

zygoloid commented 3 weeks ago

http://eel.is/c++draft/expr.const#16.sentence-3:

An aggregate initialization is an immediate invocation if it evaluates a default member initializer that has a subexpression that is an immediate-escalating expression.

So the whole initializer should be an immediate invocation.

interesting, thanks, I didn't know this form of immediate escalation existed.