[PATCH] D155175: [Clang] Fix consteval propagation for aggregates and defaulted constructors

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D155175#4498788 <https://reviews.llvm.org/D155175#4498788>, @efriedma wrote:

> Do we need CodeGen testcases here for full coverage?  The testcases from the issue passed with -fsyntax-only.
>
> --------
>
> With this patch, the following cases produce errors that don't really make sense to me:
>
>   consteval int f(int x) {
>       return x;
>   }
>   struct SS {
>       int y;
>       int x = f(this->y);
>       constexpr SS(int yy) : y(yy) {}
>   };
>   SS s = {1};
>
>
>
>   <stdin>:6:13: error: call to consteval function 'f' is not a constant expression
>       6 |     int x = f(this->y);
>         |             ^
>   <stdin>:7:15: note: in the default initializer of 'x'
>       7 |     constexpr SS(int yy) : y(yy) {}
>         |               ^
>   <stdin>:6:5: note: declared here
>       6 |     int x = f(this->y);
>         |     ^
>   <stdin>:6:15: note: use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function
>       6 |     int x = f(this->y);
>         |               ^
>   1 error generated.

This seems reasonable,  it's equivalent to  `constexpr SS(int yy) : y(yy), x(f(y)) {}` = which is not a valid call of a consteval function. And the constructor is neither defaulted, nor templated, so escalation does not happen. 
I agree that "use of 'this' pointer is only allowed within the evaluation of a call to a 'constexpr' member function" isn't amazing but it's preexisting
https://godbolt.org/z/sa78bzsed

>   consteval int f(int x) {
>       return x;
>   }
>   struct SS {
>       int y;
>       int x = f(this->y);
>       consteval SS(int yy) : y(yy) {}
>   };
>   SS s = {1};
>
>   <stdin>:6:13: error: cannot take address of consteval function 'f' outside of an immediate invocation
>       6 |     int x = f(this->y);
>         |             ^
>   <stdin>:1:15: note: declared here
>       1 | consteval int f(int x) {
>         |               ^
>   1 error generated.

Yup, this is not great - I think it's unrelated the degradation of error message was introduced between 15 and 16, I probably want to investigate in a separate patch, not sure yet.

> Somehow the following is accepted, though:
>
>   consteval int f(int x) {
>       return x;
>   }
>   struct SS {
>       int y;
>       int x = f(this->y);
>       template<typename T> constexpr SS(T yy) : y(yy) {}
>   };
>   SS s = {1};

Yes, this is bad.  Thanks for finding it.
It compiles because 1 is a constant and somehow it ends up evaluating f(1) instead of f(y) - replacing 1 by a reference to a local variable produce the expected behavior. Investigating.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D155175/new/

https://reviews.llvm.org/D155175