steveire created this revision.
steveire added reviewers: aaron.ballman, njames93.
Herald added a subscriber: xazax.hun.
steveire requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Fix handling of default construction where the constructor has a default arg.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D97142
Files:
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
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unused-raii.cpp
@@ -46,6 +46,11 @@
T();
}
+struct CtorDefaultArg {
+ CtorDefaultArg(int i = 0);
+ ~CtorDefaultArg();
+};
+
void test() {
Foo(42);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
@@ -64,6 +69,10 @@
// 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;
+
templ<FooBar>();
templ<Bar>();
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.h
@@ -28,6 +28,9 @@
}
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
Index: clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp
@@ -25,27 +25,16 @@
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")),
+ cxxConstructExpr(hasParent(compoundStmt().bind("compound")),
+ hasType(cxxRecordDecl(hasNonTrivialDestructor())))
+ .bind("expr"),
this);
}
void UnusedRaiiCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *E = Result.Nodes.getNodeAs<Expr>("expr");
+ const auto *E = Result.Nodes.getNodeAs<CXXConstructExpr>("expr");
// We ignore code expanded from macros to reduce the number of false
// positives.
@@ -55,7 +44,8 @@
// 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.
@@ -65,25 +55,17 @@
// 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 (E->getNumArgs() == 0 || isa<CXXDefaultArgExpr>(E->getArg(0))) {
+ D << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(E->getParenOrBraceRange()), 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.
- 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);
+ D << FixItHint::CreateInsertion(E->getParenOrBraceRange().getBegin(),
+ Replacement);
}
} // namespace bugprone
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
