[Sema] Fix consteval function calls with value-dependent args

[leoetlino]

Summary

If any arguments of a consteval function call are value-dependent,
the call cannot be evaluated until instantiation.

This patch fixes Sema::CheckForImmediateInvocation so we don't attempt
to evaluate consteval function calls too early (before instantiation).

This fixes things like:

consteval int f(int n) { return n; }

template <int M>
constexpr int broken() {
  return f(M);
}

Without the value-dependency checks, what happens is that the constant
expression evaluation engine is called on the following expression:

ConstantExpr 'int'
`-CallExpr 'int'
  |-ImplicitCastExpr 'int (*)(int)' <FunctionToPointerDecay>
  | `-DeclRefExpr 'int (int)' lvalue Function 'f' 'int (int)'
  `-DeclRefExpr 'int' NonTypeTemplateParm 'M' 'int'

which obviously fails when it tries to evaluate M.

[leoetlino]

While this patch doesn't introduce new regressions and does fix compilation of the code snippet mentioned earlier, I'm not sure this is the correct approach, considering there are a bunch of other similar template-related consteval bugs that this patch does *not* fix:

template <typename T>
struct Foo {
    static consteval void ok(int) {}

    static consteval void bad(T) {}

    template <typename U>
    static consteval void call(T x, U fn) { fn(x); }

    static constexpr bool test() {
        ok({});
        bad({});  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        call({}, bad);  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        return true;
    }
};

The problem seems to be that the call/arguments are type dependent so nothing is added to ImmediateInvocationCandidates -- and so HandleImmediateInvocations (in SemaExpr.cpp) never removes anything from ReferenceToConsteval despite the references to the consteval functions being used for immediate invocations.

Another variant of the issue:

template <typename T>
struct Bar {
    static consteval void bad(int) {}

    static constexpr bool test() {
        bad(T::value);  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        return true;
    }
};

Not sure what the best way of fixing those issues would be -- any guidance would be greatly appreciated!

[usaxena95]

Update: The mentioned cases here are all now fixed at head (after D132031)

// 1
consteval int f(int n) { return n; }

template <int M>
constexpr int broken() {
  return f(M);
}
static_assert(broken<100>() == 100);

// 2
template <typename T>
struct Foo {
    static consteval void ok(int) {}

    static consteval void bad(T) {}

    template <typename U>
    static consteval void call(T x, U fn) { fn(x); }

    static constexpr bool test() {
        ok({});
        bad({});  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        call({}, bad);  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        return true;
    }
};

// 3
template <typename T>
struct Bar {
    static consteval void bad(int) {}

    static constexpr bool test() {
        bad(T::value);  // error: cannot take address of consteval function 'bad' outside of an immediate invocation
        return true;
    }
};

clang/lib/Sema/SemaExpr.cpp

if (isUnevaluatedContext() || !E.isUsable() || !Decl ||
       !Decl->isConsteval() || isConstantEvaluated() ||
       RebuildingImmediateInvocation || isImmediateFunctionContext())
     return E;
 
   /// Opportunistically remove the callee from ReferencesToConsteval if we can.
   /// It's OK if this fails; we'll also remove this in
   /// HandleImmediateInvocations, but catching it here allows us to avoid
   /// walking the AST looking for it in simple cases.
-  if (auto *Call = dyn_cast<CallExpr>(E.get()->IgnoreImplicit()))
+  if (auto *Call = dyn_cast<CallExpr>(E.get()->IgnoreImplicit())) {
     if (auto *DeclRef =
             dyn_cast<DeclRefExpr>(Call->getCallee()->IgnoreImplicit()))
       ExprEvalContexts.back().ReferenceToConsteval.erase(DeclRef);
 
+    // If any arguments are value-dependent, we will not be able to evaluate
+    // the function call until instantiation.
+    if (Call->isValueDependent())
+      return E;
+
+    for (Expr *Arg : Call->arguments()) {
+      if (Arg->isValueDependent())
+        return E;
+    }
+  }
+
   E = MaybeCreateExprWithCleanups(E);
 
   ConstantExpr *Res = ConstantExpr::Create(
       getASTContext(), E.get(),
       ConstantExpr::getStorageKind(Decl->getReturnType().getTypePtr(),
                                    getASTContext()),
       /*IsImmediateInvocation*/ true);
   ExprEvalContexts.back().ImmediateInvocationCandidates.emplace_back(Res, 0);

clang/test/SemaCXX/cxx2a-consteval.cpp

struct D {
   C c;
   consteval D() = default; // expected-error {{cannot be consteval}}
   consteval ~D() = default; // expected-error {{destructor cannot be declared consteval}}
 };
 
 struct E : C {
   consteval ~E() {} // expected-error {{cannot be declared consteval}}
 };
+
+template <int X>
+constexpr int callConstevalWithNameDependentArg() {
+  return f1(X);
+}
+
+template <int X>
+constexpr int callConstevalWithNameDependentArgAsVariable() {
+  constexpr int x = X;
+  return f1(x);
+}
+
+auto foo = callConstevalWithNameDependentArg<42>();
+
 }
 
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
   return 0;
 }
 
 consteval int f_eval(int i) {
 // expected-note@-1+ {{declared here}}