[clang-tools-extra] r213647 - [clang-tidy] Add a check for RAII temporaries.
Benjamin Kramer
benny.kra at googlemail.com
Tue Jul 22 05:30:35 PDT 2014
Author: d0k
Date: Tue Jul 22 07:30:35 2014
New Revision: 213647
URL: http://llvm.org/viewvc/llvm-project?rev=213647&view=rev
Log:
[clang-tidy] Add a check for RAII temporaries.
Summary:
This tries to find code similar that immediately destroys
an object that looks like it's trying to follow RAII.
{
scoped_lock(&global_mutex);
critical_section();
}
This checker will have false positives if someone uses this pattern
to legitimately invoke a destructor immediately (or the statement is
at the end of a scope anyway). To reduce the number we ignore this
pattern in macros (this is heavily used by gtest) and ignore objects
with no user-defined destructor.
Reviewers: alexfh, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4615
Added:
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp
clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h
clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=213647&r1=213646&r2=213647&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Tue Jul 22 07:30:35 2014
@@ -6,6 +6,7 @@ add_clang_library(clangTidyMiscModule
MiscTidyModule.cpp
RedundantSmartptrGet.cpp
SwappedArgumentsCheck.cpp
+ UnusedRAII.cpp
UseOverride.cpp
LINK_LIBS
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=213647&r1=213646&r2=213647&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Tue Jul 22 07:30:35 2014
@@ -14,6 +14,7 @@
#include "BoolPointerImplicitConversion.h"
#include "RedundantSmartptrGet.h"
#include "SwappedArgumentsCheck.h"
+#include "UnusedRAII.h"
#include "UseOverride.h"
namespace clang {
@@ -35,6 +36,9 @@ public:
"misc-swapped-arguments",
new ClangTidyCheckFactory<SwappedArgumentsCheck>());
CheckFactories.addCheckFactory(
+ "misc-unused-raii",
+ new ClangTidyCheckFactory<UnusedRAIICheck>());
+ CheckFactories.addCheckFactory(
"misc-use-override",
new ClangTidyCheckFactory<UseOverride>());
}
Added: clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp?rev=213647&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp Tue Jul 22 07:30:35 2014
@@ -0,0 +1,83 @@
+//===--- UnusedRAII.cpp - clang-tidy ---------------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "UnusedRAII.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace ast_matchers {
+AST_MATCHER(CXXRecordDecl, hasUserDeclaredDestructor) {
+ // TODO: If the dtor is there but empty we don't want to warn either.
+ return Node.hasUserDeclaredDestructor();
+}
+} // namespace ast_matchers
+
+namespace tidy {
+
+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 = bindTemporaryExpr(unless(has(callExpr()))).bind("temp");
+ Finder->addMatcher(
+ exprWithCleanups(
+ unless(hasAncestor(decl(
+ anyOf(recordDecl(ast_matchers::isTemplateInstantiation()),
+ functionDecl(ast_matchers::isTemplateInstantiation()))))),
+ hasParent(compoundStmt().bind("compound")),
+ hasDescendant(typeLoc().bind("typeloc")),
+ hasType(recordDecl(hasUserDeclaredDestructor())),
+ anyOf(has(BindTemp), has(functionalCastExpr(has(BindTemp)))))
+ .bind("expr"),
+ this);
+}
+
+void UnusedRAIICheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *E = Result.Nodes.getStmtAs<Expr>("expr");
+
+ // We ignore code expanded from macros to reduce the number of false
+ // positives.
+ if (E->getLocStart().isMacroID())
+ return;
+
+ // Don't emit a warning for the last statement in the surrounding compund
+ // statement.
+ const auto *CS = Result.Nodes.getStmtAs<CompoundStmt>("compound");
+ if (E == CS->body_back())
+ return;
+
+ // Emit a warning.
+ auto D = diag(E->getLocStart(), "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.getStmtAs<CXXBindTemporaryExpr>("temp");
+ if (const auto *TOE = dyn_cast<CXXTemporaryObjectExpr>(BTE->getSubExpr()))
+ if (TOE->getNumArgs() == 0) {
+ D << FixItHint::CreateReplacement(
+ CharSourceRange::getTokenRange(TOE->getParenOrBraceRange()),
+ Replacement);
+ return;
+ }
+
+ // Otherwise just suggest adding a name.
+ const auto *TL = Result.Nodes.getNodeAs<TypeLoc>("typeloc");
+ D << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(TL->getLocEnd(), 0, *Result.SourceManager,
+ Result.Context->getLangOpts()),
+ Replacement);
+}
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h?rev=213647&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h Tue Jul 22 07:30:35 2014
@@ -0,0 +1,47 @@
+//===--- UnusedRAII.h - clang-tidy ------------------------------*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+
+/// \brief Finds temporaries that look like RAII objects.
+///
+/// The canonical example for this is a scoped lock.
+/// \code
+/// {
+/// scoped_lock(&global_mutex);
+/// critical_section();
+/// }
+/// \endcode
+/// The destructor of the scoped_lock is called before the critical_section is
+/// entered, leaving it unprotected.
+///
+/// We apply a number of heuristics to reduce the false positive count of this
+/// check:
+/// - Ignore code expanded from macros. Testing frameworks make heavy use of
+/// this.
+/// - Ignore types with no user-declared constructor. Those are very unlikely
+/// to be RAII objects.
+/// - Ignore objects at the end of a compound statement (doesn't change behavior).
+/// - Ignore objects returned from a call.
+class UnusedRAIICheck : public ClangTidyCheck {
+public:
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_UNUSED_RAII_H
Added: clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp?rev=213647&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-unused-raii.cpp Tue Jul 22 07:30:35 2014
@@ -0,0 +1,61 @@
+// RUN: $(dirname %s)/check_clang_tidy_fix.sh %s misc-unused-raii %t
+// REQUIRES: shell
+
+struct Foo {
+ Foo();
+ Foo(int);
+ Foo(int, int);
+ ~Foo();
+};
+
+struct Bar {
+ Bar();
+ Foo f;
+};
+
+template <typename T>
+void qux() {
+ T(42);
+}
+
+template <typename T>
+struct TFoo {
+ TFoo(T);
+ ~TFoo();
+};
+
+Foo f();
+
+struct Ctor {
+ Ctor(int);
+ Ctor() {
+ Ctor(0); // TODO: warn here.
+ }
+};
+
+void test() {
+ Foo(42);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+// CHECK-FIXES: Foo give_me_a_name(42);
+ Foo(23, 42);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+// CHECK-FIXES: Foo give_me_a_name(23, 42);
+ Foo();
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+// CHECK-FIXES: Foo give_me_a_name;
+ TFoo<int>(23);
+// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: object destroyed immediately after creation; did you mean to name the object?
+// CHECK-FIXES: TFoo<int> give_me_a_name(23);
+
+ Bar();
+ f();
+ qux<Foo>();
+
+#define M Foo();
+ M
+
+ {
+ Foo();
+ }
+ Foo();
+}
More information about the cfe-commits
mailing list