[clang][Sema] Skip checking int expressions for overflow in constexpr functions

[tbaeder]

Summary

I'm not 100% sure if this is crazy or not, but it doesn't even seem to break any existing test cases.

We evaluate all integer expressions for overflows and diagnose them if possible. However, in the case of a constexpr function, we will evaluate the function (and all expressions it contains) anyway after parsing it, which will check for overflow again, so the previous checking is unnecessary.

I checked that not calling CheckForIntOverflow() at all breaks tests (it does).

[shafik]

We may not have good code coverage, what about a case like this:

constexpr void f() {
    int arr[10]{};
    arr[1024*1024*1024*1204];
}

do you still obtain:

 error: constexpr function never produces a constant expression [-Winvalid-constexpr]
constexpr void f() {
               ^

godbolt: https://godbolt.org/z/5T5Gn5rxq

[tbaeder]

Output for that test case is:

./array.cpp:1:16: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
constexpr void f() {
               ^
./array.cpp:3:23: note: value 1292785156096 is outside the range of representable values of type 'int'
    arr[1024*1024*1024*1204];
                      ^
./array.cpp:3:28: warning: expression result unused [-Wunused-value]
    arr[1024*1024*1024*1204];
    ~~~ ~~~~~~~~~~~~~~~~~~~^
1 warning and 1 error generated.

The first overflow warning is missing. That seems to be new though, I don't get that warning with clang 14 either.

[aaron.ballman]

In D138270#3956221, @tbaeder wrote:

Output for that test case is:

./array.cpp:1:16: error: constexpr function never produces a constant expression [-Winvalid-constexpr]
constexpr void f() {
               ^
./array.cpp:3:23: note: value 1292785156096 is outside the range of representable values of type 'int'
    arr[1024*1024*1024*1204];
                      ^
./array.cpp:3:28: warning: expression result unused [-Wunused-value]
    arr[1024*1024*1024*1204];
    ~~~ ~~~~~~~~~~~~~~~~~~~^
1 warning and 1 error generated.

The first overflow warning is missing. That seems to be new though, I don't get that warning with clang 14 either.

The downside to losing that warning is that passing -Wno-invalid-constexpr will lose the diagnostic entirely, which seems like a regression.

clang/include/clang/Sema/Sema.h

3514–3517

You should add some documentation comments to the function.

One slight worry I have here is with dependent (perhaps special) member functions; is this function meant to answer "is this function callable in a constexpr context" or is it meant to answer "did the user specify this as a constexpr function"?

e.g.,

template <int N>
constexpr int func(int Val) { return Val / N; } // Not yet instantiated.

clang/include/clang/Sema/Sema.h

public:
   /// in the correct order.
   void EnterTemplatedContext(Scope *S, DeclContext *DC);
 
   /// Push the parameters of D, which must be a function, into scope.
   void ActOnReenterFunctionContext(Scope* S, Decl* D);
   void ActOnExitFunctionContext();
 
   /// If \p AllowLambda is true, treat lambda as function.
-  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false);
+  DeclContext *getFunctionLevelDeclContext(bool AllowLambda = false) const;
 
   /// Returns a pointer to the innermost enclosing function, or nullptr if the
   /// current context is not inside a function. If \p AllowLambda is true,
   /// this can return the call operator of an enclosing lambda, otherwise
   /// lambdas are skipped when looking for an enclosing function.
-  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false);
+  FunctionDecl *getCurFunctionDecl(bool AllowLambda = false) const;
+
+  bool curFunctionDeclIsConstexpr() const {
+    const FunctionDecl *FD = getCurFunctionDecl();
+    return FD ? FD->isConstexpr() : false;
+  }
 
   /// getCurMethodDecl - If inside of a method body, this returns a pointer to
   /// the method decl for the method being parsed.  If we're currently
   /// in a 'block', this returns the containing context.
   ObjCMethodDecl *getCurMethodDecl();
 
   /// getCurFunctionOrMethodDecl - Return the Decl for the current ObjC method
   /// or C function we're in, otherwise return null.  If we're currently

clang/lib/Sema/Sema.cpp

if (!PP.isIncrementalProcessingEnabled())
     TUScope = nullptr;
 }
 
 
 //===----------------------------------------------------------------------===//
 // Helper functions.
 //===----------------------------------------------------------------------===//
 
-DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) {
+DeclContext *Sema::getFunctionLevelDeclContext(bool AllowLambda) const {
   DeclContext *DC = CurContext;
 
   while (true) {
     if (isa<BlockDecl>(DC) || isa<EnumDecl>(DC) || isa<CapturedDecl>(DC) ||
         isa<RequiresExprBodyDecl>(DC)) {
       DC = DC->getParent();
     } else if (!AllowLambda && isa<CXXMethodDecl>(DC) &&
                cast<CXXMethodDecl>(DC)->getOverloadedOperator() == OO_Call &&
                cast<CXXRecordDecl>(DC->getParent())->isLambda()) {
       DC = DC->getParent()->getParent();
     } else break;
   }
 
   return DC;
 }
 
 /// getCurFunctionDecl - If inside of a function body, this returns a pointer
 /// to the function decl for the function being parsed.  If we're currently
 /// in a 'block', this returns the containing context.
-FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) {
+FunctionDecl *Sema::getCurFunctionDecl(bool AllowLambda) const {
   DeclContext *DC = getFunctionLevelDeclContext(AllowLambda);
   return dyn_cast<FunctionDecl>(DC);
 }
 
 ObjCMethodDecl *Sema::getCurMethodDecl() {
   DeclContext *DC = getFunctionLevelDeclContext();
   while (isa<RecordDecl>(DC))
     DC = DC->getParent();

clang/lib/Sema/SemaChecking.cpp

while (!WorkList.empty()) {
     SequenceChecker(*this, Item, WorkList);
   }
 }
 
 void Sema::CheckCompletedExpr(Expr *E, SourceLocation CheckLoc,
                               bool IsConstexpr) {
   llvm::SaveAndRestore<bool> ConstantContext(
       isConstantEvaluatedOverride, IsConstexpr || isa<ConstantExpr>(E));
+
   CheckImplicitConversions(E, CheckLoc);
   if (!E->isInstantiationDependent())
     CheckUnsequencedOperations(E);
-  if (!IsConstexpr && !E->isValueDependent())
+
+  if (!IsConstexpr && !E->isValueDependent() && !curFunctionDeclIsConstexpr())
     CheckForIntOverflow(E);
+
   DiagnoseMisalignedMembers();
 }
 
 void Sema::CheckBitFieldInitialization(SourceLocation InitLoc,
                                        FieldDecl *BitField,
                                        Expr *Init) {
   (void) AnalyzeBitFieldAssignment(*this, BitField, Init, InitLoc);
 }

clang/unittests/Support/TimeProfilerTest.cpp

constexpr int slow_init_list[] = {1, 1, 2, 3, 5, 8, 13, 21}; // 25th line
 
   setupProfiler();
   ASSERT_TRUE(compileFromString(Code, "-std=c++20", "test.cc"));
   std::string Json = teardownProfiler();
   std::string TraceGraph = buildTraceGraph(Json);
   ASSERT_TRUE(TraceGraph == R"(
 Frontend
 | EvaluateAsRValue (<test.cc:8:21>)
-| EvaluateForOverflow (<test.cc:8:21, col:25>)
 | EvaluateAsRValue (<test.cc:9:14>)
-| EvaluateForOverflow (<test.cc:9:9, col:14>)
 | isPotentialConstantExpr (slow_namespace::slow_func)
 | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)
 | | EvaluateAsRValue (<test.cc:8:21, col:25>)
 | EvaluateAsBooleanCondition (<test.cc:8:21, col:25>)
 | | EvaluateAsRValue (<test.cc:8:21, col:25>)
 | EvaluateAsInitializer (slow_value)
 | EvaluateAsConstantExpr (<test.cc:17:33, col:59>)
 | EvaluateAsConstantExpr (<test.cc:18:11, col:37>)