[clang][Interp] Don't run functions immediately after compiling them

[tbaeder]

Summary

I'm not 100% sure why this was done in the first place, but it doesn't work when the function makes assumptions about the stack of the caller, e.g. in the RVO case.

[aaron.ballman]

This function is testing whether something is potentially a constant expression. Something might not be a valid constant expression for two reasons: 1) it uses some prohibited language construct, 2) it hit undefined behavior. You don't know if you hit undefined behavior until you run the function, so could that be why the function was being run? However, I don't know how you would run an arbitrary function that might accept arguments, so the Run call does look suspicious -- especially because it landed in the initial patch of the functionality (https://reviews.llvm.org/D64146) without comment.

[tbaeder]

In D135569#3847493, @aaron.ballman wrote:

This function is testing whether something is potentially a constant expression. Something might not be a valid constant expression for two reasons: 1) it uses some prohibited language construct, 2) it hit undefined behavior. You don't know if you hit undefined behavior until you run the function, so could that be why the function was being run? However, I don't know how you would run an arbitrary function that might accept arguments, so the Run call does look suspicious -- especially because it landed in the initial patch of the functionality (https://reviews.llvm.org/D64146) without comment.

Yes. A few weeks ago I thought it might make sense to actually set functions to invalid if isPotentialConstantExpr() returns false, but found out that it returns false all the time, e.g. if the function expects parameters.

I think the whole "lets compile all constexpr functions" approach will probably go away at some point anyway? When including stdlib headers, we're probably compiling a lot of code that will never run...

[aaron.ballman]

I think this change makes sense to me. Ultimately, this API is about the potential to be constexpr, so there's no need to pay for the expense of running the function with some ginned up arguments -- if we can compile it as a constexpr function, it's potentially constexpr.

clang/lib/AST/Interp/Context.cpp

41–42

clang/lib/AST/Interp/Context.cpp

if (auto R = ByteCodeStmtGen<ByteCodeEmitter>(*this, *P).compileFunc(FD)) {
       Func = *R;
     } else {
       handleAllErrors(R.takeError(), [&Parent](ByteCodeGenError &Err) {
         Parent.FFDiag(Err.getLoc(), diag::err_experimental_clang_interp_failed);
       });
       return false;
     }
   }
 
-  if (!Func->isConstexpr())
-    return false;
-
-  APValue Dummy;
-  return Run(Parent, Func, Dummy);
+  return Func->isConstexpr();
 }
 
 bool Context::evaluateAsRValue(State &Parent, const Expr *E, APValue &Result) {
   ByteCodeExprGen<EvalEmitter> C(*this, *P, Parent, Stk, Result);
   return Check(Parent, C.interpretExpr(E));
 }
 
 bool Context::evaluateAsInitializer(State &Parent, const VarDecl *VD,