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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
