[PATCH] D155064: [clang][SemaCXX] Diagnose tautological uses of consteval if and is_constant_evaluated

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/test/SemaCXX/vartemplate-lambda.cpp:17
+                                                        // expected-note{{cannot be used in a constant expression}} \
+                                                        // expected-error 2{{a lambda expression may not appear inside of a constant expression}}
 };
----------------
hazohelet wrote:
> cor3ntin wrote:
> > hazohelet wrote:
> > > cor3ntin wrote:
> > > > This also looks like a regression.
> > > > 
> > > > The current error is much clearer, can you investigate?
> > > > ```
> > > > <source>:3:22: error: constexpr variable 't<int>' must be initialized by a constant expression
> > > >     3 |   static constexpr T t = [](int f = T(7)){return f;}();
> > > >       |                      ^   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > > <source>:6:12: note: in instantiation of static data member 'S::t<int>' requested here
> > > >     6 | int a = S::t<int>;
> > > >       |            ^
> > > > <source>:3:26: note: non-literal type 'S::(lambda at <source>:3:26)' cannot be used in a constant expression
> > > >     3 |   static constexpr T t = [](int f = T(7)){return f;}();
> > > >       |                          ^
> > > > ```
> > > > 
> > > > Why do we emt 2 errors instead of a single note? Here the error is that the initializer is not a constant expression, everything else should be notes.
> > > "lambda cannot be in constant expression" error is emitted from Sema against lambda expressions in constant-evaluated contexts in C++14 mode, and the note is emitted from constexpr evaluator.
> > > The Sema-side error is emitted twice because it is emitted both before/after instantiation.
> > > We can suppress one of them by ignoring it when sema is instantiating variable template initializer.
> > > Or we can completely suppress this Sema error against initializers to avoid duplicate errors from Sema and constexpr evaluator.
> > > I think "lambda cannot be in constant expression" Sema error is more understandable than the constexpr evaluator note "non-literal type cannot be in constant expression", so I think it is ok to keep one Sema error here.
> > So maybe the issue is that we are not making the declaration invalid in sema when we get this error? Can you look into it?
> > any opinion @aaron.ballman 
> I updated the patch to keep a single sema error here and stop constant interpreter from evaluating the initializer by marking declaration invalid. I like having one sema error here.
I'll wait a bit to approve because I'd like a second opinion, but otherwise I think we are in pretty good shape in this patch!


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

https://reviews.llvm.org/D155064