[clang] Check captured variables for noreturn attribute #155213
+48
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
|
@llvm/pr-subscribers-clang Author: Serge Pavlov (spavloff) ChangesAnaysis 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:
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}}
|
You can test this locally with the following command:git-clang-format --diff HEAD~1 HEAD --extensions cpp -- clang/lib/Sema/AnalysisBasedWarnings.cpp clang/test/SemaCXX/noreturn-vars.cppView the diff from clang-format here.diff --git a/clang/lib/Sema/AnalysisBasedWarnings.cpp b/clang/lib/Sema/AnalysisBasedWarnings.cpp
index fe1498003..e92536b56 100644
--- a/clang/lib/Sema/AnalysisBasedWarnings.cpp
+++ b/clang/lib/Sema/AnalysisBasedWarnings.cpp
@@ -495,13 +495,9 @@ struct TransferFunctions : public StmtVisitor<TransferFunctions> {
Visit(MTE->getSubExpr());
}
- void VisitExprWithCleanups(FullExpr *FE) {
- Visit(FE->getSubExpr());
- }
+ void VisitExprWithCleanups(FullExpr *FE) { Visit(FE->getSubExpr()); }
- void VisitCXXConstructExpr(CXXConstructExpr *CE) {
- Visit(CE->getArg(0));
- }
+ void VisitCXXConstructExpr(CXXConstructExpr *CE) { Visit(CE->getArg(0)); }
};
} // namespace
|
erichkeane
reviewed
Aug 25, 2025
|
|
||
| [[noreturn]] void test_lambda_var(int x) { | ||
| func_type func_ptr = noret; | ||
| auto LF = [&](){func_ptr = ordinary;}; |
There was a problem hiding this comment.
Would like to see a test for member functions here as well. Also, test that shows capture-by-value doesn't cause this diagnostic.
| } | ||
|
|
||
| void VisitCXXConstructExpr(CXXConstructExpr *CE) { | ||
| Visit(CE->getArg(0)); |
There was a problem hiding this comment.
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?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.