njames93 created this revision.
njames93 added reviewers: aaron.ballman, alexfh, LegalizeAdulthood, gribozavr2,
dodohand.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Fixed a false positive where a lambda expression in the condition which
contained an assignement would trigger a warning.
Fixes #56729
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D132786
Files:
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-assignment-in-if-condition.cpp
clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/assignment-in-if-condition.cpp
@@ -101,3 +101,20 @@
f = 5;
}
}
+
+template <typename Func> bool exec(Func F) { return F(); }
+
+void lambda_if() {
+ int X;
+ if ([&X] {
+ X = 5;
+ return true;
+ }()) {
+ }
+
+ if (exec([&] {
+ X = 5;
+ return true;
+ })) {
+ }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -116,6 +116,10 @@
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Fixed a false positive in :doc:`bugprone-assignment-in-if-condition
+ <clang-tidy/checks/bugprone/assignment-in-if-condition>` check when there
+ was an assignement in a lambda found in the condition of an ``if``.
+
- Improved :doc:`bugprone-signal-handler
<clang-tidy/checks/bugprone/signal-handler>` check. Partial
support for C++14 signal handler rules was added. Bug report generation was
Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.h
@@ -25,6 +25,7 @@
: ClangTidyCheck(Name, Context) {}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void report(const Expr *E);
};
} // namespace bugprone
Index: clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/AssignmentInIfConditionCheck.cpp
@@ -8,7 +8,10 @@
#include "AssignmentInIfConditionCheck.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/ExprCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
using namespace clang::ast_matchers;
@@ -17,23 +20,50 @@
namespace bugprone {
void AssignmentInIfConditionCheck::registerMatchers(MatchFinder *Finder) {
- Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
- binaryOperator(isAssignmentOperator())
- .bind("assignment_in_if_statement")))),
- this);
- Finder->addMatcher(ifStmt(hasCondition(forEachDescendant(
- cxxOperatorCallExpr(isAssignmentOperator())
- .bind("assignment_in_if_statement")))),
- this);
+ Finder->addMatcher(translationUnitDecl(), this);
}
void AssignmentInIfConditionCheck::check(
- const MatchFinder::MatchResult &Result) {
- const auto *MatchedDecl =
- Result.Nodes.getNodeAs<clang::Stmt>("assignment_in_if_statement");
- if (!MatchedDecl) {
- return;
- }
+ const ast_matchers::MatchFinder::MatchResult &Result) {
+ class Visitor : public RecursiveASTVisitor<Visitor> {
+ AssignmentInIfConditionCheck &Check;
+
+ public:
+ explicit Visitor(AssignmentInIfConditionCheck &Check) : Check(Check) {}
+ bool VisitIfStmt(IfStmt *If) {
+ class ConditionVisitor : public RecursiveASTVisitor<ConditionVisitor> {
+ AssignmentInIfConditionCheck &Check;
+
+ public:
+ explicit ConditionVisitor(AssignmentInIfConditionCheck &Check)
+ : Check(Check) {}
+
+ // Dont traverse into any lambda expressions.
+ bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+ return true;
+ }
+
+ bool VisitBinaryOperator(BinaryOperator *BO) {
+ if (BO->isAssignmentOp())
+ Check.report(BO);
+ return true;
+ }
+
+ bool VisitCXXOperatorCallExpr(CXXOperatorCallExpr *OCE) {
+ if (OCE->isAssignmentOp())
+ Check.report(OCE);
+ return true;
+ }
+ };
+
+ ConditionVisitor(Check).TraverseStmt(If->getCond());
+ return true;
+ }
+ };
+ Visitor(*this).TraverseAST(*Result.Context);
+}
+
+void AssignmentInIfConditionCheck::report(const Expr *MatchedDecl) {
diag(MatchedDecl->getBeginLoc(),
"an assignment within an 'if' condition is bug-prone");
diag(MatchedDecl->getBeginLoc(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
