[clang-tools-extra] r213738 - Reapply r213647 with a fix.
David Blaikie
dblaikie at gmail.com
Wed Jul 23 08:26:13 PDT 2014
On Wed, Jul 23, 2014 at 4:49 AM, Benjamin Kramer
<benny.kra at googlemail.com> wrote:
> Author: d0k
> Date: Wed Jul 23 06:49:46 2014
> New Revision: 213738
>
> URL: http://llvm.org/viewvc/llvm-project?rev=213738&view=rev
> Log:
> Reapply r213647 with a fix.
>
> ASTMatchers currently have problems mixing bound TypeLoc nodes with Decl/Stmt
> nodes. That should be fixed soon but for this checker there we only need the
> TypeLoc to generate a fixit so postpone the potentially heavyweight AST walking
> until after we know that we're going to emit a warning.
>
> This is covered by existing test cases.
Though this failure didn't turn up for you locally before the original
commit? (does it turn up on all buildbots, and not on local builds for
some reason?) Is there any test we could add that would help
demonstrate this failure sooner/more pervasively (usually if we see,
say, a architecture-specific failure we add an test with the specified
architecture hardcoded so that it reproduces locally for everyone
rather than relying on the coverage of a particular buildbot)
>
> Original message:
> [clang-tidy] Add a check for RAII temporaries.
>
> 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.
>
> 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=213738&r1=213737&r2=213738&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
> +++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Wed Jul 23 06:49:46 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=213738&r1=213737&r2=213738&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Wed Jul 23 06:49:46 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=213738&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp (added)
> +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.cpp Wed Jul 23 06:49:46 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=213738&view=auto
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h (added)
> +++ clang-tools-extra/trunk/clang-tidy/misc/UnusedRAII.h Wed Jul 23 06:49:46 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=213738&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 Wed Jul 23 06:49:46 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();
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list