[clang] Check captured variables for noreturn attribute

[spavloff]

Anaysis for noreturn attribute now misses the case when an assignment to a variable is made inside a lambda function call. This change fixes that case.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Serge Pavlov (spavloff)

Changes

Anaysis for noreturn attribute now misses the case when an assignment to a variable is made inside a lambda function call. This change fixes that case.

---

Full diff: https://github.com/llvm/llvm-project/pull/155213.diff

2 Files Affected:

• (modified) clang/lib/Sema/AnalysisBasedWarnings.cpp (+35)
• (modified) clang/test/SemaCXX/noreturn-vars.cpp (+13)

diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index 0b94b1044f072..fe1498003e7da 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -467,6 +467,41 @@ struct TransferFunctions : public StmtVisitor<TransferFunctions> {
               AllValuesAreNoReturn = false;
     }
   }
+
+  void VisitCXXOperatorCallExpr(CXXOperatorCallExpr *CE) {
+    if (CE->getOperator() == OO_Call && CE->getNumArgs() > 0) {
+      Expr *Obj = CE->getArg(0)->IgnoreParenCasts();
+      if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Obj))
+        Obj = MTE->getSubExpr();
+      if (auto *DRE = dyn_cast<DeclRefExpr>(Obj)) {
+        auto *D = dyn_cast<VarDecl>(DRE->getDecl());
+        if (D->hasInit())
+          Obj = D->getInit();
+      }
+      Visit(Obj);
+    }
+  }
+
+  void VisitLambdaExpr(LambdaExpr *LE) {
+    for (const LambdaCapture &Capture : LE->captures())
+      if (Capture.capturesVariable())
+        if (const VarDecl *VD = dyn_cast<VarDecl>(Capture.getCapturedVar()))
+          if (VD == Var)
+            if (Capture.getCaptureKind() == LCK_ByRef)
+              AllValuesAreNoReturn = false;
+  }
+
+  void VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *MTE) {
+    Visit(MTE->getSubExpr());
+  }
+
+  void VisitExprWithCleanups(FullExpr *FE) {
+    Visit(FE->getSubExpr());
+  }
+
+  void VisitCXXConstructExpr(CXXConstructExpr *CE) {
+    Visit(CE->getArg(0));
+  }
 };
 } // namespace
 
diff --git a/clang/test/SemaCXX/noreturn-vars.cpp b/clang/test/SemaCXX/noreturn-vars.cpp
index ca65fcf5ca31d..3b2473c1bc670 100644
--- a/clang/test/SemaCXX/noreturn-vars.cpp
+++ b/clang/test/SemaCXX/noreturn-vars.cpp
@@ -225,3 +225,16 @@ extern void abc_02(func_type *);
   abc_02(&func_ptr);
   func_ptr();
 } // expected-warning {{function declared 'noreturn' should not return}}
+
+[[noreturn]] void test_lambda() {
+  func_type func_ptr = noret;
+  [&func_ptr]() { func_ptr = ordinary; } ();
+  func_ptr();
+} // expected-warning {{function declared 'noreturn' should not return}}
+
+[[noreturn]] void test_lambda_var(int x) {
+  func_type func_ptr = noret;
+  auto LF = [&](){func_ptr = ordinary;};
+  LF();
+  func_ptr();
+} // expected-warning {{function declared 'noreturn' should not return}}

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[erichkeane]

Would like to see a test for member functions here as well. Also, test that shows capture-by-value doesn't cause this diagnostic.

[spavloff]

The added test lambda_capture_by_value checks capture by value. As for member functions, capture of this as in the example:

class LambdaTest_1 {
  func_type func_ptr = noret;  
  [[noreturn]] void method() {
    auto LF = [this](){ return func_ptr; };
    LF();
    func_ptr();
  } // expected-warning {{function declared 'noreturn' should not return}}
};

the captured variables are represented by member expressions rather that references to variables This case requires more complex analysis that the current, it can be implemented in future.

[erichkeane]

This doesn't seem right at all. the 'this' argument in a CXXConstructExpr does NOT get placed in the 0 arg (in fact, they are not present), so you can't assume 0 exists here. AND it isn't really clear why the 0 arg is important here, but others arent?

[spavloff]

It is more about the way LambdaExpr can be reached from variable initializer. For example, the code:

auto LF = [&func_ptr]() { func_ptr = ordinary; };

produces AST like:

DeclStmt 0x129e8159a10 <line:10:3, col:72>
`-VarDecl 0x129e812a478 <col:3, col:71> col:8 used LF '(lambda at zz-01.cpp:10:13)' cinit
  `-ExprWithCleanups 0x129e81599f8 <col:13, col:71> '(lambda at zz-01.cpp:10:13)'
    `-CXXConstructExpr 0x129e81599c8 <col:13, col:71> '(lambda at zz-01.cpp:10:13)' 'void ((lambda at zz-01.cpp:10:13) &&) noexcept' elidable
      `-MaterializeTemporaryExpr 0x129e8159728 <col:13, col:71> '(lambda at zz-01.cpp:10:13)' xvalue
        `-LambdaExpr 0x129e812aa78 <col:13, col:71> '(lambda at zz-01.cpp:10:13)'

These 'Visit*' methods are introduced to support this kind of lambda usage.

[spavloff]

Ping.