[Clang][Sema] Do not evaluate value-dependent immediate invocations

[Izaron]

Summary

Value-dependent ConstantExprs are not meant to be evaluated.
There is an assert in Expr::EvaluateAsConstantExpr that ensures this condition.
But before this patch the method was called without prior check.

Fixes https://github.com/llvm/llvm-project/issues/52768

[Izaron]

The assert that I mentioned in the summary: https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ExprConstant.cpp#L14969
(Although asserts are deleted in release builds, one can check the violation by llvm::errs() << isValueDependent() << "\n"; in the method)

I think I have found the right place to check the condition, right after creating the ConstantExpr object.

P.S. If this review is eventually approved, kindly please merge the commit on my behalf =) As I don't have merge access. My name is Evgeny Shulgin and email is [e-mail]. Sorry for inconvenience!

[cor3ntin]

This looks like the right fix to me but I would prefer @aaron.ballman or @erichkeane to have the final say.

clang/lib/Sema/SemaExpr.cpp

16820–16823

I'd say something like

/ Value-dependent constant expressionS should not be immediately
evaluated until they are instantiated.

[Izaron]

Fix comment

[Izaron]

clang/lib/Sema/SemaExpr.cpp

16820–16823

Sure, thanks! I'm bad at writing comments =)

[erichkeane]

1 nit, but otherwise I'm happy with this.

clang/test/SemaCXX/cxx2a-consteval.cpp

616

A quick comment on this test as to why it has no diagnostics would be nice.

[Izaron]

Add test comments

[Izaron]

A friendly ping =) Seems like I don't have write access, so unfortunately I have to ask people to merge commits on my behalf. Let me copy-paste the usual comment of my reviews:

P.S. If this review is eventually approved, kindly please merge the commit on my behalf =) As I don't have merge access. My name is Evgeny Shulgin and email is [e-mail]. Sorry for inconvenience!

[cor3ntin]

In D119375#3329014, @Izaron wrote:

A friendly ping =) Seems like I don't have write access, so unfortunately I have to ask people to merge commits on my behalf. Let me copy-paste the usual comment of my reviews:

I landed your change.
Thanks for the patch!

clang/lib/Sema/SemaExpr.cpp

ExprResult Sema::CheckForImmediateInvocation(ExprResult E, FunctionDecl *Decl) {
 
   E = MaybeCreateExprWithCleanups(E);
 
   ConstantExpr *Res = ConstantExpr::Create(
       getASTContext(), E.get(),
       ConstantExpr::getStorageKind(Decl->getReturnType().getTypePtr(),
                                    getASTContext()),
       /*IsImmediateInvocation*/ true);
+  /// Value-dependent constant expressions should not be immediately
+  /// evaluated until they are instantiated.
+  if (!Res->isValueDependent())
     ExprEvalContexts.back().ImmediateInvocationCandidates.emplace_back(Res, 0);
   return Res;
 }
 
 static void EvaluateAndDiagnoseImmediateInvocation(
     Sema &SemaRef, Sema::ImmediateInvocationCandidate Candidate) {
   llvm::SmallVector<PartialDiagnosticAt, 8> Notes;
   Expr::EvalResult Eval;
   Eval.Diag = &Notes;

clang/test/SemaCXX/cxx2a-consteval.cpp

 auto e = g(f()); // expected-error {{is not a constant expression}}
                  // expected-note@-1 {{non-constexpr function 'f' cannot be used in a constant expression}}
 
 using T = decltype(g(f()));
 static_assert(is_same<long, T>::value);
 
 } // namespace unevaluated
 
+namespace value_dependent {
+
+consteval int foo(int x) {
+  return x;
+}
+
+template <int X> constexpr int bar() {
+  // Previously this call was rejected as value-dependent constant expressions
+  // can't be immediately evaluated. Now we show that we don't immediately
+  // evaluate them until they are instantiated.
+  return foo(X);
+}
+
+template <typename T> constexpr int baz() {
+  constexpr int t = sizeof(T);
+  // Previously this call was rejected as `t` is value-dependent and its value
+  // is unknown until the function is instantiated. Now we show that we don't
+  // reject such calls.
+  return foo(t);
+}
+
+static_assert(bar<15>() == 15);
+static_assert(baz<int>() == sizeof(int));
+
+} // namespace value_dependent
+
 namespace PR50779 {
 struct derp {
   int b = 0;
 };
 
 constexpr derp d;
 
 struct test {