[clang-tools-extra] 6ccb806 - [clang-tidy] Extract areStatementsIdentical
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Mon May 15 13:05:28 PDT 2023
Author: Piotr Zegar
Date: 2023-05-15T20:05:12Z
New Revision: 6ccb8061724fb6ee0b2598764656410ce3f2472c
URL: https://github.com/llvm/llvm-project/commit/6ccb8061724fb6ee0b2598764656410ce3f2472c
DIFF: https://github.com/llvm/llvm-project/commit/6ccb8061724fb6ee0b2598764656410ce3f2472c.diff
LOG: [clang-tidy] Extract areStatementsIdentical
Move areStatementsIdentical from BranchCloneCheck into ASTUtils.
Add small improvments. Use it in LoopConvertUtils.
Reviewed By: carlosgalvezp
Differential Revision: https://reviews.llvm.org/D148995
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
clang-tools-extra/clang-tidy/utils/ASTUtils.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
index df232e9a64cd0..7657ca7380b6f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "BranchCloneCheck.h"
+#include "../utils/ASTUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
#include "clang/Analysis/CloneDetection.h"
@@ -16,26 +17,6 @@
using namespace clang;
using namespace clang::ast_matchers;
-/// Returns true when the statements are Type I clones of each other.
-static bool areStatementsIdentical(const Stmt *LHS, const Stmt *RHS,
- const ASTContext &Context) {
- if (isa<Expr>(LHS) && isa<Expr>(RHS)) {
- // If we have errors in expressions, we will be unable
- // to accurately profile and compute hashes for each
- // of the left and right statements.
- const auto *LHSExpr = llvm::cast<Expr>(LHS);
- const auto *RHSExpr = llvm::cast<Expr>(RHS);
- if (LHSExpr->containsErrors() && RHSExpr->containsErrors()) {
- return false;
- }
- }
-
- llvm::FoldingSetNodeID DataLHS, DataRHS;
- LHS->Profile(DataLHS, Context, false);
- RHS->Profile(DataRHS, Context, false);
- return (DataLHS == DataRHS);
-}
-
namespace {
/// A branch in a switch may consist of several statements; while a branch in
/// an if/else if/else chain is one statement (which may be a CompoundStmt).
@@ -55,8 +36,9 @@ static bool areSwitchBranchesIdentical(const SwitchBranch LHS,
// NOTE: We strip goto labels and annotations in addition to stripping
// the `case X:` or `default:` labels, but it is very unlikely that this
// would cause false positives in real-world code.
- if (!areStatementsIdentical(LHS[I]->stripLabelLikeStatements(),
- RHS[I]->stripLabelLikeStatements(), Context)) {
+ if (!tidy::utils::areStatementsIdentical(LHS[I]->stripLabelLikeStatements(),
+ RHS[I]->stripLabelLikeStatements(),
+ Context)) {
return false;
}
}
@@ -89,8 +71,8 @@ void BranchCloneCheck::check(const MatchFinder::MatchResult &Result) {
if (!isa<IfStmt>(Else)) {
// Just a simple if with no `else if` branch.
- if (areStatementsIdentical(Then->IgnoreContainers(),
- Else->IgnoreContainers(), Context)) {
+ if (utils::areStatementsIdentical(Then->IgnoreContainers(),
+ Else->IgnoreContainers(), Context)) {
diag(IS->getBeginLoc(), "if with identical then and else branches");
diag(IS->getElseLoc(), "else branch starts here", DiagnosticIDs::Note);
}
@@ -130,9 +112,9 @@ void BranchCloneCheck::check(const MatchFinder::MatchResult &Result) {
int NumCopies = 1;
for (size_t J = I + 1; J < N; J++) {
- if (KnownAsClone[J] ||
- !areStatementsIdentical(Branches[I]->IgnoreContainers(),
- Branches[J]->IgnoreContainers(), Context))
+ if (KnownAsClone[J] || !utils::areStatementsIdentical(
+ Branches[I]->IgnoreContainers(),
+ Branches[J]->IgnoreContainers(), Context))
continue;
NumCopies++;
@@ -160,7 +142,8 @@ void BranchCloneCheck::check(const MatchFinder::MatchResult &Result) {
if (const auto *CO = Result.Nodes.getNodeAs<ConditionalOperator>("condOp")) {
// We do not try to detect chains of ?: operators.
- if (areStatementsIdentical(CO->getTrueExpr(), CO->getFalseExpr(), Context))
+ if (utils::areStatementsIdentical(CO->getTrueExpr(), CO->getFalseExpr(),
+ Context))
diag(CO->getQuestionLoc(),
"conditional operator with identical true and false expressions");
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
index deb82b4947bbc..295c2446f49db 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "LoopConvertUtils.h"
+#include "../utils/ASTUtils.h"
#include "clang/Basic/IdentifierTable.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Lambda.h"
@@ -190,13 +191,7 @@ const Expr *digThroughConstructorsConversions(const Expr *E) {
/// Returns true when two Exprs are equivalent.
bool areSameExpr(ASTContext *Context, const Expr *First, const Expr *Second) {
- if (!First || !Second)
- return false;
-
- llvm::FoldingSetNodeID FirstID, SecondID;
- First->Profile(FirstID, *Context, true);
- Second->Profile(SecondID, *Context, true);
- return FirstID == SecondID;
+ return utils::areStatementsIdentical(First, Second, *Context, true);
}
/// Returns the DeclRefExpr represented by E, or NULL if there isn't one.
diff --git a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
index 9fda73e9047bb..64333f2c18745 100644
--- a/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
+++ b/clang-tools-extra/clang-tidy/utils/ASTUtils.cpp
@@ -88,4 +88,29 @@ bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM) {
!utils::rangeContainsMacroExpansion(Range, SM);
}
+bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt,
+ const ASTContext &Context, bool Canonical) {
+ if (!FirstStmt || !SecondStmt)
+ return false;
+
+ if (FirstStmt == SecondStmt)
+ return true;
+
+ if (FirstStmt->getStmtClass() != FirstStmt->getStmtClass())
+ return false;
+
+ if (isa<Expr>(FirstStmt) && isa<Expr>(SecondStmt)) {
+ // If we have errors in expressions, we will be unable
+ // to accurately profile and compute hashes for each statements.
+ if (llvm::cast<Expr>(FirstStmt)->containsErrors() ||
+ llvm::cast<Expr>(SecondStmt)->containsErrors())
+ return false;
+ }
+
+ llvm::FoldingSetNodeID DataFirst, DataSecond;
+ FirstStmt->Profile(DataFirst, Context, Canonical);
+ SecondStmt->Profile(DataSecond, Context, Canonical);
+ return DataFirst == DataSecond;
+}
+
} // namespace clang::tidy::utils
diff --git a/clang-tools-extra/clang-tidy/utils/ASTUtils.h b/clang-tools-extra/clang-tidy/utils/ASTUtils.h
index 21bdfff0d73ff..1bba5daf2fc76 100644
--- a/clang-tools-extra/clang-tidy/utils/ASTUtils.h
+++ b/clang-tools-extra/clang-tidy/utils/ASTUtils.h
@@ -36,6 +36,10 @@ bool rangeContainsMacroExpansion(SourceRange Range, const SourceManager *SM);
// FIXME: false-negative if the entire range is fully expanded from a macro.
bool rangeCanBeFixed(SourceRange Range, const SourceManager *SM);
+// Check if statements are same
+bool areStatementsIdentical(const Stmt *FirstStmt, const Stmt *SecondStmt,
+ const ASTContext &Context, bool Canonical = false);
+
} // namespace clang::tidy::utils
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ASTUTILS_H
More information about the cfe-commits
mailing list