[clang-tools-extra] 7b6fc9a - [clang-tidy] Simplify unused RAII check
Stephen Kelly via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 2 13:37:38 PST 2021
Author: Stephen Kelly
Date: 2021-03-02T21:33:34Z
New Revision: 7b6fc9a1055a7eae07645fb7d3085dc89a8d54ab
URL: https://github.com/llvm/llvm-project/commit/7b6fc9a1055a7eae07645fb7d3085dc89a8d54ab
DIFF: https://github.com/llvm/llvm-project/commit/7b6fc9a1055a7eae07645fb7d3085dc89a8d54ab.diff
LOG: [clang-tidy] Simplify unused RAII check
Fix handling of default construction where the constructor has a default arg.
Differential Revision: https://reviews.llvm.org/D97142
Added:
Modified:
clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
index 3a52180cf04d..b87bb9e8ca95 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,25 +25,37 @@ AST_MATCHER(CXXRecordDecl, hasNonTrivialDestructor) {
void UnusedRaiiCheck::registerMatchers(MatchFinder *Finder) {
// Look for temporaries that are constructed in-place and immediately
- // destroyed. Look for temporaries created by a functional cast but not for
- // those returned from a call.
- auto BindTemp = cxxBindTemporaryExpr(
- unless(has(ignoringParenImpCasts(callExpr()))),
- unless(has(ignoringParenImpCasts(objcMessageExpr()))))
- .bind("temp");
+ // destroyed.
Finder->addMatcher(
- traverse(TK_AsIs,
- exprWithCleanups(
- unless(isInTemplateInstantiation()),
- hasParent(compoundStmt().bind("compound")),
- hasType(cxxRecordDecl(hasNonTrivialDestructor())),
- anyOf(has(ignoringParenImpCasts(BindTemp)),
- has(ignoringParenImpCasts(cxxFunctionalCastExpr(
- has(ignoringParenImpCasts(BindTemp)))))))
- .bind("expr")),
+ mapAnyOf(cxxConstructExpr, cxxUnresolvedConstructExpr)
+ .with(hasParent(compoundStmt().bind("compound")),
+ anyOf(hasType(cxxRecordDecl(hasNonTrivialDestructor())),
+ hasType(templateSpecializationType(
+ hasDeclaration(classTemplateDecl(has(
+ cxxRecordDecl(hasNonTrivialDestructor()))))))))
+ .bind("expr"),
this);
}
+template <typename T>
+void reportDiagnostic(DiagnosticBuilder D, const T *Node, SourceRange SR,
+ bool DefaultConstruction) {
+ const char *Replacement = " give_me_a_name";
+
+ // If this is a default ctor we have to remove the parens or we'll introduce a
+ // most vexing parse.
+ if (DefaultConstruction) {
+ D << FixItHint::CreateReplacement(CharSourceRange::getTokenRange(SR),
+ Replacement);
+ return;
+ }
+
+ // Otherwise just suggest adding a name. To find the place to insert the name
+ // find the first TypeLoc in the children of E, which always points to the
+ // written type.
+ D << FixItHint::CreateInsertion(SR.getBegin(), Replacement);
+}
+
void UnusedRaiiCheck::check(const MatchFinder::MatchResult &Result) {
const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
@@ -55,35 +67,32 @@ void UnusedRaiiCheck::check(const MatchFinder::MatchResult &Result) {
// Don't emit a warning for the last statement in the surrounding compound
// statement.
const auto *CS = Result.Nodes.getNodeAs<CompoundStmt>("compound");
- if (E == CS->body_back())
+ const auto *LastExpr = dyn_cast<Expr>(CS->body_back());
+
+ if (LastExpr && E == LastExpr->IgnoreUnlessSpelledInSource())
return;
// Emit a warning.
auto D = diag(E->getBeginLoc(), "object destroyed immediately after "
"creation; did you mean to name the object?");
- const char *Replacement = " give_me_a_name";
- // If this is a default ctor we have to remove the parens or we'll introduce a
- // most vexing parse.
- const auto *BTE = Result.Nodes.getNodeAs<CXXBindTemporaryExpr>("temp");
- if (const auto *TOE = dyn_cast<CXXTemporaryObjectExpr>(BTE->getSubExpr()))
- if (TOE->getNumArgs() == 0) {
- D << FixItHint::CreateReplacement(
- CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()),
- Replacement);
- return;
+ if (const auto *Node = dyn_cast<CXXConstructExpr>(E))
+ reportDiagnostic(D, Node, Node->getParenOrBraceRange(),
+ Node->getNumArgs() == 0 ||
+ isa<CXXDefaultArgExpr>(Node->getArg(0)));
+ if (const auto *Node = dyn_cast<CXXUnresolvedConstructExpr>(E)) {
+ auto SR = SourceRange(Node->getLParenLoc(), Node->getRParenLoc());
+ auto DefaultConstruction = Node->getNumArgs() == 0;
+ if (!DefaultConstruction) {
+ auto FirstArg = Node->getArg(0);
+ DefaultConstruction = isa<CXXDefaultArgExpr>(FirstArg);
+ if (auto ILE = dyn_cast<InitListExpr>(FirstArg)) {
+ DefaultConstruction = ILE->getNumInits() == 0;
+ SR = SourceRange(ILE->getLBraceLoc(), ILE->getRBraceLoc());
+ }
}
-
- // Otherwise just suggest adding a name. To find the place to insert the name
- // find the first TypeLoc in the children of E, which always points to the
- // written type.
- auto Matches =
- match(expr(hasDescendant(typeLoc().bind("t"))), *E, *Result.Context);
- if (const auto *TL = selectFirst<TypeLoc>("t", Matches))
- D << FixItHint::CreateInsertion(
- Lexer::getLocForEndOfToken(TL->getEndLoc(), 0, *Result.SourceManager,
- getLangOpts()),
- Replacement);
+ reportDiagnostic(D, Node, SR, DefaultConstruction);
+ }
}
} // namespace bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
index 360836586699..f4a0de02d39a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
@@ -28,6 +28,9 @@ class UnusedRaiiCheck : public ClangTidyCheck {
}
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ llvm::Optional<TraversalKind> getCheckTraversalKind() const override {
+ return TK_IgnoreUnlessSpelledInSource;
+ }
};
} // namespace bugprone
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
index 3428d453c63d..94643676f45c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s bugprone-unused-raii %t
+// RUN: %check_clang_tidy %s bugprone-unused-raii %t -- -- -fno-delayed-template-parsing
struct Foo {
Foo();
@@ -46,6 +46,42 @@ void neverInstantiated() {
T();
}
+struct CtorDefaultArg {
+ CtorDefaultArg(int i = 0);
+ ~CtorDefaultArg();
+};
+
+template <typename T>
+struct TCtorDefaultArg {
+ TCtorDefaultArg(T i = 0);
+ ~TCtorDefaultArg();
+};
+
+struct CtorTwoDefaultArg {
+ CtorTwoDefaultArg(int i = 0, bool b = false);
+ ~CtorTwoDefaultArg();
+};
+
+template <typename T>
+void templatetest() {
+ TCtorDefaultArg<T>();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<T> give_me_a_name;
+ TCtorDefaultArg<T>{};
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<T> give_me_a_name;
+
+ TCtorDefaultArg<T>(T{});
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<T> give_me_a_name(T{});
+ TCtorDefaultArg<T>{T{}};
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<T> give_me_a_name{T{}};
+
+ int i = 0;
+ (void)i;
+}
+
void test() {
Foo(42);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
@@ -71,6 +107,22 @@ void test() {
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
// CHECK-FIXES: FooBar give_me_a_name;
+ CtorDefaultArg();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: CtorDefaultArg give_me_a_name;
+
+ CtorTwoDefaultArg();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: CtorTwoDefaultArg give_me_a_name;
+
+ TCtorDefaultArg<int>();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<int> give_me_a_name;
+
+ TCtorDefaultArg<int>{};
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+ // CHECK-FIXES: TCtorDefaultArg<int> give_me_a_name;
+
templ<FooBar>();
templ<Bar>();
More information about the cfe-commits
mailing list