alexfh created this revision. alexfh added reviewers: bkramer, klimek, rsmith. Herald added subscribers: jdoerfert, jsji, kbarton, nemanjai. Herald added a project: clang.
When a clang tool runs on a translation unit that uses modular headers, no PPCallbacks will be invoked for the code in the modular headers (since they are not parsed, but loaded as AST). Since some tools depend on PPCallbacks for all transitively included headers, a solution is to re-lex the modular headers from the sources stored along with AST inside module files. This patch makes it possible to re-attach PPCallbacks from the compiler's Preprocessor to a custom one that will be used to re-lex all transitively included headers. It also updates clang-tidy checks to be compatible with this mode of operation. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D58278 Files: clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp clang-tools-extra/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp clang/include/clang/Lex/PPCallbacks.h clang/include/clang/Lex/Preprocessor.h
Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -908,11 +908,19 @@ /// it. PPCallbacks *getPPCallbacks() const { return Callbacks.get(); } void addPPCallbacks(std::unique_ptr<PPCallbacks> C) { + assert(C && "Callback cannot be null."); if (Callbacks) C = llvm::make_unique<PPChainedCallbacks>(std::move(C), std::move(Callbacks)); + C->setPreprocessor(this); Callbacks = std::move(C); } + + std::unique_ptr<PPCallbacks> takePPCallbacks() { + if (Callbacks) + Callbacks->setPreprocessor(nullptr); + return std::move(Callbacks); + } /// \} bool isMacroDefined(StringRef Id) { Index: clang/include/clang/Lex/PPCallbacks.h =================================================================== --- clang/include/clang/Lex/PPCallbacks.h +++ clang/include/clang/Lex/PPCallbacks.h @@ -27,6 +27,7 @@ class MacroDefinition; class MacroDirective; class MacroArgs; + class Preprocessor; /// This interface provides a way to observe the actions of the /// preprocessor as it does its thing. @@ -341,6 +342,16 @@ /// \param IfLoc the source location of the \#if/\#ifdef/\#ifndef directive. virtual void Endif(SourceLocation Loc, SourceLocation IfLoc) { } + + virtual void setPreprocessor(Preprocessor *preprocessor) { + preprocessor_ = preprocessor; + } + +protected: + Preprocessor *getPreprocessor() const { return preprocessor_; } + +private: + Preprocessor *preprocessor_; }; /// Simple wrapper class for chaining callbacks. @@ -556,6 +567,12 @@ First->Endif(Loc, IfLoc); Second->Endif(Loc, IfLoc); } + + void setPreprocessor(Preprocessor *preprocessor) override { + PPCallbacks::setPreprocessor(preprocessor); + First->setPreprocessor(preprocessor); + Second->setPreprocessor(preprocessor); + } }; } // end namespace clang Index: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp =================================================================== --- clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp +++ clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp @@ -27,15 +27,14 @@ namespace { class HeaderGuardPPCallbacks : public PPCallbacks { public: - HeaderGuardPPCallbacks(Preprocessor *PP, HeaderGuardCheck *Check) - : PP(PP), Check(Check) {} + HeaderGuardPPCallbacks(HeaderGuardCheck *Check) : Check(Check) {} void FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, FileID PrevFID) override { // Record all files we enter. We'll need them to diagnose headers without // guards. - SourceManager &SM = PP->getSourceManager(); + SourceManager &SM = getPreprocessor()->getSourceManager(); if (Reason == EnterFile && FileType == SrcMgr::C_User) { if (const FileEntry *FE = SM.getFileEntryForID(SM.getFileID(Loc))) { std::string FileName = cleanPath(FE->getName()); @@ -68,7 +67,7 @@ void EndOfMainFile() override { // Now that we have all this information from the preprocessor, use it! - SourceManager &SM = PP->getSourceManager(); + SourceManager &SM = getPreprocessor()->getSourceManager(); for (const auto &MacroEntry : Macros) { const MacroInfo *MI = MacroEntry.second; @@ -136,7 +135,8 @@ size_t *EndIfLenPtr = nullptr) { if (!EndIf.isValid()) return false; - const char *EndIfData = PP->getSourceManager().getCharacterData(EndIf); + const char *EndIfData = + getPreprocessor()->getSourceManager().getCharacterData(EndIf); size_t EndIfLen = std::strcspn(EndIfData, "\r\n"); if (EndIfLenPtr) *EndIfLenPtr = EndIfLen; @@ -214,7 +214,7 @@ if (!Check->shouldSuggestToAddHeaderGuard(FileName)) continue; - SourceManager &SM = PP->getSourceManager(); + SourceManager &SM = getPreprocessor()->getSourceManager(); FileID FID = SM.translateFile(FE.getValue()); SourceLocation StartLoc = SM.getLocForStartOfFile(FID); if (StartLoc.isInvalid()) @@ -261,15 +261,13 @@ Ifndefs; std::map<SourceLocation, SourceLocation> EndIfs; - Preprocessor *PP; HeaderGuardCheck *Check; }; } // namespace void HeaderGuardCheck::registerPPCallbacks(CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - llvm::make_unique<HeaderGuardPPCallbacks>(&Compiler.getPreprocessor(), - this)); + llvm::make_unique<HeaderGuardPPCallbacks>(this)); } bool HeaderGuardCheck::shouldSuggestEndifComment(StringRef FileName) { Index: clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp +++ clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp @@ -25,9 +25,8 @@ enum DirectiveKind { DK_If = 0, DK_Ifdef = 1, DK_Ifndef = 2 }; public: - explicit RedundantPreprocessorCallbacks(ClangTidyCheck &Check, - Preprocessor &PP) - : Check(Check), PP(PP), + explicit RedundantPreprocessorCallbacks(ClangTidyCheck &Check) + : Check(Check), WarningDescription("nested redundant %select{#if|#ifdef|#ifndef}0; " "consider removing it"), NoteDescription("previous %select{#if|#ifdef|#ifndef}0 was here") {} @@ -36,13 +35,14 @@ ConditionValueKind ConditionValue) override { StringRef Condition = Lexer::getSourceText(CharSourceRange::getTokenRange(ConditionRange), - PP.getSourceManager(), PP.getLangOpts()); + getPreprocessor()->getSourceManager(), + getPreprocessor()->getLangOpts()); CheckMacroRedundancy(Loc, Condition, IfStack, DK_If, DK_If, true); } void Ifdef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MacroDefinition) override { - std::string MacroName = PP.getSpelling(MacroNameTok); + std::string MacroName = getPreprocessor()->getSpelling(MacroNameTok); CheckMacroRedundancy(Loc, MacroName, IfdefStack, DK_Ifdef, DK_Ifdef, true); CheckMacroRedundancy(Loc, MacroName, IfndefStack, DK_Ifdef, DK_Ifndef, false); @@ -50,7 +50,7 @@ void Ifndef(SourceLocation Loc, const Token &MacroNameTok, const MacroDefinition &MacroDefinition) override { - std::string MacroName = PP.getSpelling(MacroNameTok); + std::string MacroName = getPreprocessor()->getSpelling(MacroNameTok); CheckMacroRedundancy(Loc, MacroName, IfndefStack, DK_Ifndef, DK_Ifndef, true); CheckMacroRedundancy(Loc, MacroName, IfdefStack, DK_Ifndef, DK_Ifdef, @@ -71,7 +71,7 @@ SmallVector<PreprocessorEntry, 4> &Stack, DirectiveKind WarningKind, DirectiveKind NoteKind, bool Store) { - if (PP.getSourceManager().isInMainFile(Loc)) { + if (getPreprocessor()->getSourceManager().isInMainFile(Loc)) { for (const auto &Entry : Stack) { if (Entry.Condition == MacroName) { Check.diag(Loc, WarningDescription) << WarningKind; @@ -87,7 +87,6 @@ } ClangTidyCheck &Check; - Preprocessor &PP; SmallVector<PreprocessorEntry, 4> IfStack; SmallVector<PreprocessorEntry, 4> IfdefStack; SmallVector<PreprocessorEntry, 4> IfndefStack; @@ -99,8 +98,7 @@ void RedundantPreprocessorCheck::registerPPCallbacks( CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - ::llvm::make_unique<RedundantPreprocessorCallbacks>( - *this, Compiler.getPreprocessor())); + ::llvm::make_unique<RedundantPreprocessorCallbacks>(*this)); } } // namespace readability Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp +++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp @@ -138,14 +138,14 @@ /// Callback supplies macros to IdentifierNamingCheck::checkMacro class IdentifierNamingCheckPPCallbacks : public PPCallbacks { public: - IdentifierNamingCheckPPCallbacks(Preprocessor *PP, - IdentifierNamingCheck *Check) - : PP(PP), Check(Check) {} + IdentifierNamingCheckPPCallbacks(IdentifierNamingCheck *Check) + : Check(Check) {} /// MacroDefined calls checkMacro for macros in the main file void MacroDefined(const Token &MacroNameTok, const MacroDirective *MD) override { - Check->checkMacro(PP->getSourceManager(), MacroNameTok, MD->getMacroInfo()); + Check->checkMacro(getPreprocessor()->getSourceManager(), MacroNameTok, + MD->getMacroInfo()); } /// MacroExpands calls expandMacro for macros in the main file @@ -156,7 +156,6 @@ } private: - Preprocessor *PP; IdentifierNamingCheck *Check; }; } // namespace @@ -242,8 +241,7 @@ void IdentifierNamingCheck::registerPPCallbacks(CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - llvm::make_unique<IdentifierNamingCheckPPCallbacks>( - &Compiler.getPreprocessor(), this)); + llvm::make_unique<IdentifierNamingCheckPPCallbacks>(this)); } static bool matchesStyle(StringRef Name, Index: clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp +++ clang-tools-extra/clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp @@ -33,8 +33,8 @@ class AvoidUnderscoreInGoogletestNameCallback : public PPCallbacks { public: AvoidUnderscoreInGoogletestNameCallback( - Preprocessor *PP, AvoidUnderscoreInGoogletestNameCheck *Check) - : PP(PP), Check(Check) {} + AvoidUnderscoreInGoogletestNameCheck *Check) + : Check(Check) {} // Detects expansions of the TEST, TEST_F, TEST_P, TYPED_TEST, TYPED_TEST_P // macros and checks that their arguments do not have any underscores. @@ -52,14 +52,16 @@ const Token *TestNameToken = Args->getUnexpArgument(1); if (!TestCaseNameToken || !TestNameToken) return; - std::string TestCaseName = PP->getSpelling(*TestCaseNameToken); + std::string TestCaseName = + getPreprocessor()->getSpelling(*TestCaseNameToken); if (TestCaseName.find('_') != std::string::npos) Check->diag(TestCaseNameToken->getLocation(), "avoid using \"_\" in test case name \"%0\" according to " "Googletest FAQ") << TestCaseName; - std::string TestNameMaybeDisabled = PP->getSpelling(*TestNameToken); + std::string TestNameMaybeDisabled = + getPreprocessor()->getSpelling(*TestNameToken); StringRef TestName = TestNameMaybeDisabled; TestName.consume_front(kDisabledTestPrefix); if (TestName.contains('_')) @@ -70,7 +72,6 @@ } private: - Preprocessor *PP; AvoidUnderscoreInGoogletestNameCheck *Check; }; @@ -79,8 +80,7 @@ void AvoidUnderscoreInGoogletestNameCheck::registerPPCallbacks( CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - llvm::make_unique<AvoidUnderscoreInGoogletestNameCallback>( - &Compiler.getPreprocessor(), this)); + llvm::make_unique<AvoidUnderscoreInGoogletestNameCallback>(this)); } } // namespace readability Index: clang-tools-extra/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/MacroRepeatedSideEffectsCheck.cpp @@ -19,15 +19,13 @@ namespace { class MacroRepeatedPPCallbacks : public PPCallbacks { public: - MacroRepeatedPPCallbacks(ClangTidyCheck &Check, Preprocessor &PP) - : Check(Check), PP(PP) {} + MacroRepeatedPPCallbacks(ClangTidyCheck &Check) : Check(Check) {} void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, SourceRange Range, const MacroArgs *Args) override; private: ClangTidyCheck &Check; - Preprocessor &PP; unsigned countArgumentExpansions(const MacroInfo *MI, const IdentifierInfo *Arg) const; @@ -146,7 +144,8 @@ // If another macro is found within the macro definition, skip the macro // and the eventual arguments. if (TII->hasMacroDefinition()) { - const MacroInfo *M = PP.getMacroDefinition(TII).getMacroInfo(); + const MacroInfo *M = + getPreprocessor()->getMacroDefinition(TII).getMacroInfo(); if (M != nullptr && M->isFunctionLike()) SkipParen = true; continue; @@ -174,8 +173,7 @@ void MacroRepeatedSideEffectsCheck::registerPPCallbacks( CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - ::llvm::make_unique<MacroRepeatedPPCallbacks>( - *this, Compiler.getPreprocessor())); + ::llvm::make_unique<MacroRepeatedPPCallbacks>(*this)); } } // namespace bugprone Index: clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp @@ -18,8 +18,7 @@ namespace { class MacroParenthesesPPCallbacks : public PPCallbacks { public: - MacroParenthesesPPCallbacks(Preprocessor *PP, MacroParenthesesCheck *Check) - : PP(PP), Check(Check) {} + MacroParenthesesPPCallbacks(MacroParenthesesCheck *Check) : Check(Check) {} void MacroDefined(const Token &MacroNameTok, const MacroDirective *MD) override { @@ -34,7 +33,6 @@ /// Arguments should be enclosed in parentheses. void argument(const Token &MacroNameTok, const MacroInfo *MI); - Preprocessor *PP; MacroParenthesesCheck *Check; }; } // namespace @@ -146,9 +144,10 @@ const Token &Last = *(MI->tokens_end() - 1); Check->diag(Loc, "macro replacement list should be enclosed in parentheses") << FixItHint::CreateInsertion(MI->tokens_begin()->getLocation(), "(") - << FixItHint::CreateInsertion(Last.getLocation().getLocWithOffset( - PP->getSpelling(Last).length()), - ")"); + << FixItHint::CreateInsertion( + Last.getLocation().getLocWithOffset( + getPreprocessor()->getSpelling(Last).length()), + ")"); } } @@ -227,7 +226,8 @@ continue; // C++ template parameters. - if (PP->getLangOpts().CPlusPlus && Prev.isOneOf(tok::comma, tok::less) && + if (getPreprocessor()->getLangOpts().CPlusPlus && + Prev.isOneOf(tok::comma, tok::less) && Next.isOneOf(tok::comma, tok::greater)) continue; @@ -242,16 +242,16 @@ Check->diag(Tok.getLocation(), "macro argument should be enclosed in " "parentheses") << FixItHint::CreateInsertion(Tok.getLocation(), "(") - << FixItHint::CreateInsertion(Tok.getLocation().getLocWithOffset( - PP->getSpelling(Tok).length()), - ")"); + << FixItHint::CreateInsertion( + Tok.getLocation().getLocWithOffset( + getPreprocessor()->getSpelling(Tok).length()), + ")"); } } void MacroParenthesesCheck::registerPPCallbacks(CompilerInstance &Compiler) { Compiler.getPreprocessor().addPPCallbacks( - llvm::make_unique<MacroParenthesesPPCallbacks>( - &Compiler.getPreprocessor(), this)); + llvm::make_unique<MacroParenthesesPPCallbacks>(this)); } } // namespace bugprone
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits