[clang-tools-extra] r311020 - [clang-tidy] Use CloexecCheck as base class.
Evgenii Stepanov via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 16 13:39:44 PDT 2017
Hi,
this change breaks build:
clang-tools-extra/clang-tidy/android/CloexecSocketCheck.cpp:20:30:
error: unused variable 'SOCK_CLOEXEC'
[-Werror,-Wunused-const-variable]
static constexpr const char *SOCK_CLOEXEC = "SOCK_CLOEXEC";
Please test with LLVM_ENABLE_WERROR=ON before submitting!
On Wed, Aug 16, 2017 at 9:59 AM, Chih-Hung Hsieh via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> Author: chh
> Date: Wed Aug 16 09:59:26 2017
> New Revision: 311020
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311020&view=rev
> Log:
> [clang-tidy] Use CloexecCheck as base class.
>
> Summary:
> Simplify registerMatchers and check functions in CloexecCreatCheck,
> CloexecSocketCheck, CloexecFopenCheck, and CloexecOpenCheck.
>
> Differential Revision: https://reviews.llvm.org/D36761
>
>
> Modified:
> clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.cpp
> clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.h
> clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.cpp
> clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.h
> clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.cpp
> clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.h
> clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp
> clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.h
> clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp
> clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.cpp?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.cpp Wed Aug 16 09:59:26 2017
> @@ -20,10 +20,6 @@ namespace tidy {
> namespace android {
>
> namespace {
> -
> -const char *const FuncDeclBindingStr = "funcDecl";
> -const char *const FuncBindingStr = "func";
> -
> // Helper function to form the correct string mode for Type3.
> // Build the replace text. If it's string constant, add <Mode> directly in the
> // end of the string. Else, add <Mode>.
> @@ -41,6 +37,10 @@ std::string buildFixMsgForStringFlag(con
> }
> } // namespace
>
> +constexpr char CloexecCheck::FuncDeclBindingStr[];
> +
> +constexpr char CloexecCheck::FuncBindingStr[];
> +
> void CloexecCheck::registerMatchersImpl(
> MatchFinder *Finder, internal::Matcher<FunctionDecl> Function) {
> // We assume all the checked APIs are C functions.
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.h?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecCheck.h Wed Aug 16 09:59:26 2017
> @@ -90,6 +90,12 @@ protected:
> /// Helper function to get the spelling of a particular argument.
> StringRef getSpellingArg(const ast_matchers::MatchFinder::MatchResult &Result,
> int N) const;
> +
> + /// Binding name of the FuncDecl of a function call.
> + static constexpr char FuncDeclBindingStr[] = "funcDecl";
> +
> + /// Binding name of the function call expression.
> + static constexpr char FuncBindingStr[] = "func";
> };
>
> } // namespace android
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.cpp?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.cpp Wed Aug 16 09:59:26 2017
> @@ -10,7 +10,6 @@
> #include "CloexecCreatCheck.h"
> #include "clang/AST/ASTContext.h"
> #include "clang/ASTMatchers/ASTMatchFinder.h"
> -#include "clang/Lex/Lexer.h"
>
> using namespace clang::ast_matchers;
>
> @@ -21,37 +20,22 @@ namespace android {
> void CloexecCreatCheck::registerMatchers(MatchFinder *Finder) {
> auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
> auto MODETType = hasType(namedDecl(hasName("mode_t")));
> -
> - Finder->addMatcher(
> - callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
> - hasName("creat"),
> - hasParameter(0, CharPointerType),
> - hasParameter(1, MODETType))
> - .bind("funcDecl")))
> - .bind("creatFn"),
> - this);
> + registerMatchersImpl(Finder,
> + functionDecl(isExternC(), returns(isInteger()),
> + hasName("creat"),
> + hasParameter(0, CharPointerType),
> + hasParameter(1, MODETType)));
> }
>
> void CloexecCreatCheck::check(const MatchFinder::MatchResult &Result) {
> - const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("creatFn");
> - const SourceManager &SM = *Result.SourceManager;
> -
> const std::string &ReplacementText =
> - (Twine("open (") +
> - Lexer::getSourceText(CharSourceRange::getTokenRange(
> - MatchedCall->getArg(0)->getSourceRange()),
> - SM, Result.Context->getLangOpts()) +
> + (Twine("open (") + getSpellingArg(Result, 0) +
> ", O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC, " +
> - Lexer::getSourceText(CharSourceRange::getTokenRange(
> - MatchedCall->getArg(1)->getSourceRange()),
> - SM, Result.Context->getLangOpts()) +
> - ")")
> + getSpellingArg(Result, 1) + ")")
> .str();
> -
> - diag(MatchedCall->getLocStart(),
> - "prefer open() to creat() because open() allows O_CLOEXEC")
> - << FixItHint::CreateReplacement(MatchedCall->getSourceRange(),
> - ReplacementText);
> + replaceFunc(Result,
> + "prefer open() to creat() because open() allows O_CLOEXEC",
> + ReplacementText);
> }
>
> } // namespace android
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.h?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecCreatCheck.h Wed Aug 16 09:59:26 2017
> @@ -10,7 +10,7 @@
> #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_CREAT_H
> #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_CREAT_H
>
> -#include "../ClangTidy.h"
> +#include "CloexecCheck.h"
>
> namespace clang {
> namespace tidy {
> @@ -20,10 +20,10 @@ namespace android {
> /// Find the usage of creat() and redirect user to use open().
>
> /// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-creat.html
> -class CloexecCreatCheck : public ClangTidyCheck {
> +class CloexecCreatCheck : public CloexecCheck {
> public:
> CloexecCreatCheck(StringRef Name, ClangTidyContext *Context)
> - : ClangTidyCheck(Name, Context) {}
> + : CloexecCheck(Name, Context) {}
> void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
> };
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.cpp?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.cpp Wed Aug 16 09:59:26 2017
> @@ -18,55 +18,17 @@ namespace clang {
> namespace tidy {
> namespace android {
>
> -namespace {
> -static const char MODE = 'e';
> -
> -// Build the replace text. If it's string constant, add 'e' directly in the end
> -// of the string. Else, add "e".
> -std::string BuildReplaceText(const Expr *Arg, const SourceManager &SM,
> - const LangOptions &LangOpts) {
> - if (Arg->getLocStart().isMacroID())
> - return (Lexer::getSourceText(
> - CharSourceRange::getTokenRange(Arg->getSourceRange()), SM,
> - LangOpts) +
> - " \"" + Twine(MODE) + "\"")
> - .str();
> -
> - StringRef SR = cast<StringLiteral>(Arg->IgnoreParenCasts())->getString();
> - return ("\"" + SR + Twine(MODE) + "\"").str();
> -}
> -} // namespace
> -
> void CloexecFopenCheck::registerMatchers(MatchFinder *Finder) {
> auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
> -
> - Finder->addMatcher(
> - callExpr(callee(functionDecl(isExternC(), returns(asString("FILE *")),
> - hasName("fopen"),
> - hasParameter(0, CharPointerType),
> - hasParameter(1, CharPointerType))
> - .bind("funcDecl")))
> - .bind("fopenFn"),
> - this);
> + registerMatchersImpl(Finder,
> + functionDecl(isExternC(), returns(asString("FILE *")),
> + hasName("fopen"),
> + hasParameter(0, CharPointerType),
> + hasParameter(1, CharPointerType)));
> }
>
> void CloexecFopenCheck::check(const MatchFinder::MatchResult &Result) {
> - const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("fopenFn");
> - const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
> - const Expr *ModeArg = MatchedCall->getArg(1);
> -
> - // Check if the 'e' may be in the mode string.
> - const auto *ModeStr = dyn_cast<StringLiteral>(ModeArg->IgnoreParenCasts());
> - if (!ModeStr || (ModeStr->getString().find(MODE) != StringRef::npos))
> - return;
> -
> - const std::string &ReplacementText = BuildReplaceText(
> - ModeArg, *Result.SourceManager, Result.Context->getLangOpts());
> -
> - diag(ModeArg->getLocStart(), "use %0 mode 'e' to set O_CLOEXEC")
> - << FD
> - << FixItHint::CreateReplacement(ModeArg->getSourceRange(),
> - ReplacementText);
> + insertStringFlag(Result, /*Mode=*/'e', /*ArgPos=*/1);
> }
>
> } // namespace android
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.h?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecFopenCheck.h Wed Aug 16 09:59:26 2017
> @@ -10,7 +10,7 @@
> #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H
> #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_FOPEN_H
>
> -#include "../ClangTidy.h"
> +#include "CloexecCheck.h"
>
> namespace clang {
> namespace tidy {
> @@ -23,10 +23,10 @@ namespace android {
> /// constant propagation.
> ///
> /// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-fopen.html
> -class CloexecFopenCheck : public ClangTidyCheck {
> +class CloexecFopenCheck : public CloexecCheck {
> public:
> CloexecFopenCheck(StringRef Name, ClangTidyContext *Context)
> - : ClangTidyCheck(Name, Context) {}
> + : CloexecCheck(Name, Context) {}
> void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
> };
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.cpp Wed Aug 16 09:59:26 2017
> @@ -8,10 +8,8 @@
> //===----------------------------------------------------------------------===//
>
> #include "CloexecOpenCheck.h"
> -#include "../utils/ASTUtils.h"
> #include "clang/AST/ASTContext.h"
> #include "clang/ASTMatchers/ASTMatchFinder.h"
> -#include "clang/Lex/Lexer.h"
>
> using namespace clang::ast_matchers;
>
> @@ -19,54 +17,26 @@ namespace clang {
> namespace tidy {
> namespace android {
>
> -static constexpr const char *O_CLOEXEC = "O_CLOEXEC";
> -
> void CloexecOpenCheck::registerMatchers(MatchFinder *Finder) {
> auto CharPointerType = hasType(pointerType(pointee(isAnyCharacter())));
> -
> - Finder->addMatcher(
> - callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
> - hasAnyName("open", "open64"),
> - hasParameter(0, CharPointerType),
> - hasParameter(1, hasType(isInteger())))
> - .bind("funcDecl")))
> - .bind("openFn"),
> - this);
> - Finder->addMatcher(
> - callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
> - hasName("openat"),
> - hasParameter(0, hasType(isInteger())),
> - hasParameter(1, CharPointerType),
> - hasParameter(2, hasType(isInteger())))
> - .bind("funcDecl")))
> - .bind("openatFn"),
> - this);
> + registerMatchersImpl(Finder,
> + functionDecl(isExternC(), returns(isInteger()),
> + hasAnyName("open", "open64"),
> + hasParameter(0, CharPointerType),
> + hasParameter(1, hasType(isInteger()))));
> + registerMatchersImpl(Finder,
> + functionDecl(isExternC(), returns(isInteger()),
> + hasName("openat"),
> + hasParameter(0, hasType(isInteger())),
> + hasParameter(1, CharPointerType),
> + hasParameter(2, hasType(isInteger()))));
> }
>
> void CloexecOpenCheck::check(const MatchFinder::MatchResult &Result) {
> - const Expr *FlagArg = nullptr;
> - if (const auto *OpenFnCall = Result.Nodes.getNodeAs<CallExpr>("openFn"))
> - FlagArg = OpenFnCall->getArg(1);
> - else if (const auto *OpenFnCall =
> - Result.Nodes.getNodeAs<CallExpr>("openatFn"))
> - FlagArg = OpenFnCall->getArg(2);
> - assert(FlagArg);
> -
> - const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
> -
> - // Check the required flag.
> - SourceManager &SM = *Result.SourceManager;
> - if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM,
> - Result.Context->getLangOpts(), O_CLOEXEC))
> - return;
> -
> - SourceLocation EndLoc =
> - Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
> - Result.Context->getLangOpts());
> -
> - diag(EndLoc, "%0 should use %1 where possible")
> - << FD << O_CLOEXEC
> - << FixItHint::CreateInsertion(EndLoc, (Twine(" | ") + O_CLOEXEC).str());
> + const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>(FuncDeclBindingStr);
> + assert(FD->param_size() > 1);
> + int ArgPos = (FD->param_size() > 2) ? 2 : 1;
> + insertMacroFlag(Result, /*MarcoFlag=*/"O_CLOEXEC", ArgPos);
> }
>
> } // namespace android
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.h?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecOpenCheck.h Wed Aug 16 09:59:26 2017
> @@ -10,7 +10,7 @@
> #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
> #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_OPEN_H
>
> -#include "../ClangTidy.h"
> +#include "CloexecCheck.h"
>
> namespace clang {
> namespace tidy {
> @@ -25,10 +25,10 @@ namespace android {
> ///
> /// Only the symbolic 'O_CLOEXEC' macro definition is checked, not the concrete
> /// value.
> -class CloexecOpenCheck : public ClangTidyCheck {
> +class CloexecOpenCheck : public CloexecCheck {
> public:
> CloexecOpenCheck(StringRef Name, ClangTidyContext *Context)
> - : ClangTidyCheck(Name, Context) {}
> + : CloexecCheck(Name, Context) {}
> void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
> };
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.cpp Wed Aug 16 09:59:26 2017
> @@ -8,7 +8,6 @@
> //===----------------------------------------------------------------------===//
>
> #include "CloexecSocketCheck.h"
> -#include "../utils/ASTUtils.h"
> #include "clang/AST/ASTContext.h"
> #include "clang/ASTMatchers/ASTMatchFinder.h"
>
> @@ -21,35 +20,16 @@ namespace android {
> static constexpr const char *SOCK_CLOEXEC = "SOCK_CLOEXEC";
>
> void CloexecSocketCheck::registerMatchers(MatchFinder *Finder) {
> - Finder->addMatcher(
> - callExpr(callee(functionDecl(isExternC(), returns(isInteger()),
> - hasName("socket"),
> - hasParameter(0, hasType(isInteger())),
> - hasParameter(1, hasType(isInteger())),
> - hasParameter(2, hasType(isInteger())))
> - .bind("funcDecl")))
> - .bind("socketFn"),
> - this);
> + registerMatchersImpl(Finder,
> + functionDecl(isExternC(), returns(isInteger()),
> + hasName("socket"),
> + hasParameter(0, hasType(isInteger())),
> + hasParameter(1, hasType(isInteger())),
> + hasParameter(2, hasType(isInteger()))));
> }
>
> void CloexecSocketCheck::check(const MatchFinder::MatchResult &Result) {
> - const auto *MatchedCall = Result.Nodes.getNodeAs<CallExpr>("socketFn");
> - const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("funcDecl");
> - const Expr *FlagArg = MatchedCall->getArg(1);
> - SourceManager &SM = *Result.SourceManager;
> -
> - if (utils::exprHasBitFlagWithSpelling(FlagArg->IgnoreParenCasts(), SM,
> - Result.Context->getLangOpts(), SOCK_CLOEXEC))
> - return;
> -
> - SourceLocation EndLoc =
> - Lexer::getLocForEndOfToken(SM.getFileLoc(FlagArg->getLocEnd()), 0, SM,
> - Result.Context->getLangOpts());
> -
> - diag(EndLoc, "%0 should use %1 where possible")
> - << FD << SOCK_CLOEXEC
> - << FixItHint::CreateInsertion(EndLoc,
> - (Twine(" | ") + SOCK_CLOEXEC).str());
> + insertMacroFlag(Result, /*MarcoFlag=*/"SOCK_CLOEXEC", /*ArgPos=*/1);
> }
>
> } // namespace android
>
> Modified: clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h
> URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h?rev=311020&r1=311019&r2=311020&view=diff
> ==============================================================================
> --- clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h (original)
> +++ clang-tools-extra/trunk/clang-tidy/android/CloexecSocketCheck.h Wed Aug 16 09:59:26 2017
> @@ -10,7 +10,7 @@
> #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_SOCKET_H
> #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ANDROID_CLOEXEC_SOCKET_H
>
> -#include "../ClangTidy.h"
> +#include "CloexecCheck.h"
>
> namespace clang {
> namespace tidy {
> @@ -20,10 +20,10 @@ namespace android {
> ///
> /// For the user-facing documentation see:
> /// http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-socket.html
> -class CloexecSocketCheck : public ClangTidyCheck {
> +class CloexecSocketCheck : public CloexecCheck {
> public:
> CloexecSocketCheck(StringRef Name, ClangTidyContext *Context)
> - : ClangTidyCheck(Name, Context) {}
> + : CloexecCheck(Name, Context) {}
> void registerMatchers(ast_matchers::MatchFinder *Finder) override;
> void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
> };
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list