llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Edwin Vane (revane) <details> <summary>Changes</summary> Some functions allow a null SourceManager, no SourceManager, or a SourceManager in an inconsistent argument position. Since SourceManager is generally not null and it doesn't make sense to apply renaming without one, these inconsistencies are now gone. --- Full diff: https://github.com/llvm/llvm-project/pull/88268.diff 2 Files Affected: - (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp (+26-20) - (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h (+6-5) ``````````diff diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp index ad8048e2a92b7e..962a243ce94d48 100644 --- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -169,14 +169,14 @@ class RenamerClangTidyCheckPPCallbacks : public PPCallbacks { return; if (SM.isWrittenInCommandLineFile(MacroNameTok.getLocation())) return; - Check->checkMacro(SM, MacroNameTok, Info); + Check->checkMacro(MacroNameTok, Info, SM); } /// MacroExpands calls expandMacro for macros in the main file void MacroExpands(const Token &MacroNameTok, const MacroDefinition &MD, SourceRange /*Range*/, const MacroArgs * /*Args*/) override { - Check->expandMacro(MacroNameTok, MD.getMacroInfo()); + Check->expandMacro(MacroNameTok, MD.getMacroInfo(), SM); } private: @@ -187,7 +187,7 @@ class RenamerClangTidyCheckPPCallbacks : public PPCallbacks { class RenamerClangTidyVisitor : public RecursiveASTVisitor<RenamerClangTidyVisitor> { public: - RenamerClangTidyVisitor(RenamerClangTidyCheck *Check, const SourceManager *SM, + RenamerClangTidyVisitor(RenamerClangTidyCheck *Check, const SourceManager &SM, bool AggressiveDependentMemberLookup) : Check(Check), SM(SM), AggressiveDependentMemberLookup(AggressiveDependentMemberLookup) {} @@ -258,7 +258,7 @@ class RenamerClangTidyVisitor // Fix overridden methods if (const auto *Method = dyn_cast<CXXMethodDecl>(Decl)) { if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) { - Check->addUsage(Overridden, Method->getLocation()); + Check->addUsage(Overridden, Method->getLocation(), SM); return true; // Don't try to add the actual decl as a Failure. } } @@ -268,7 +268,7 @@ class RenamerClangTidyVisitor if (isa<ClassTemplateSpecializationDecl>(Decl)) return true; - Check->checkNamedDecl(Decl, *SM); + Check->checkNamedDecl(Decl, SM); return true; } @@ -385,7 +385,7 @@ class RenamerClangTidyVisitor private: RenamerClangTidyCheck *Check; - const SourceManager *SM; + const SourceManager &SM; const bool AggressiveDependentMemberLookup; }; @@ -415,7 +415,7 @@ void RenamerClangTidyCheck::registerPPCallbacks( void RenamerClangTidyCheck::addUsage( const RenamerClangTidyCheck::NamingCheckId &Decl, SourceRange Range, - const SourceManager *SourceMgr) { + const SourceManager &SourceMgr) { // Do nothing if the provided range is invalid. if (Range.isInvalid()) return; @@ -425,8 +425,7 @@ void RenamerClangTidyCheck::addUsage( // spelling location to different source locations, and we only want to fix // the token once, before it is expanded by the macro. SourceLocation FixLocation = Range.getBegin(); - if (SourceMgr) - FixLocation = SourceMgr->getSpellingLoc(FixLocation); + FixLocation = SourceMgr.getSpellingLoc(FixLocation); if (FixLocation.isInvalid()) return; @@ -440,15 +439,15 @@ void RenamerClangTidyCheck::addUsage( if (!Failure.shouldFix()) return; - if (SourceMgr && SourceMgr->isWrittenInScratchSpace(FixLocation)) + if (SourceMgr.isWrittenInScratchSpace(FixLocation)) Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro; - if (!utils::rangeCanBeFixed(Range, SourceMgr)) + if (!utils::rangeCanBeFixed(Range, &SourceMgr)) Failure.FixStatus = RenamerClangTidyCheck::ShouldFixStatus::InsideMacro; } void RenamerClangTidyCheck::addUsage(const NamedDecl *Decl, SourceRange Range, - const SourceManager *SourceMgr) { + const SourceManager &SourceMgr) { // Don't keep track for non-identifier names. auto *II = Decl->getIdentifier(); if (!II) @@ -489,18 +488,24 @@ void RenamerClangTidyCheck::checkNamedDecl(const NamedDecl *Decl, } Failure.Info = std::move(Info); - addUsage(Decl, Range, &SourceMgr); + addUsage(Decl, Range, SourceMgr); } void RenamerClangTidyCheck::check(const MatchFinder::MatchResult &Result) { - RenamerClangTidyVisitor Visitor(this, Result.SourceManager, + if (!Result.SourceManager) { + // In principle SourceManager is not null but going only by the definition + // of MatchResult it must be handled. Cannot rename anything without a + // SourceManager. + return; + } + RenamerClangTidyVisitor Visitor(this, *Result.SourceManager, AggressiveDependentMemberLookup); Visitor.TraverseAST(*Result.Context); } -void RenamerClangTidyCheck::checkMacro(const SourceManager &SourceMgr, - const Token &MacroNameTok, - const MacroInfo *MI) { +void RenamerClangTidyCheck::checkMacro(const Token &MacroNameTok, + const MacroInfo *MI, + const SourceManager &SourceMgr) { std::optional<FailureInfo> MaybeFailure = getMacroFailureInfo(MacroNameTok, SourceMgr); if (!MaybeFailure) @@ -515,11 +520,12 @@ void RenamerClangTidyCheck::checkMacro(const SourceManager &SourceMgr, Failure.FixStatus = ShouldFixStatus::FixInvalidIdentifier; Failure.Info = std::move(Info); - addUsage(ID, Range); + addUsage(ID, Range, SourceMgr); } void RenamerClangTidyCheck::expandMacro(const Token &MacroNameTok, - const MacroInfo *MI) { + const MacroInfo *MI, + const SourceManager &SourceMgr) { StringRef Name = MacroNameTok.getIdentifierInfo()->getName(); NamingCheckId ID(MI->getDefinitionLoc(), Name); @@ -528,7 +534,7 @@ void RenamerClangTidyCheck::expandMacro(const Token &MacroNameTok, return; SourceRange Range(MacroNameTok.getLocation(), MacroNameTok.getEndLoc()); - addUsage(ID, Range); + addUsage(ID, Range, SourceMgr); } static std::string diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h index 38228fb59bf624..be5b6f0c7f7678 100644 --- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h +++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h @@ -108,18 +108,19 @@ class RenamerClangTidyCheck : public ClangTidyCheck { llvm::DenseMap<NamingCheckId, NamingCheckFailure>; /// Check Macros for style violations. - void checkMacro(const SourceManager &SourceMgr, const Token &MacroNameTok, - const MacroInfo *MI); + void checkMacro(const Token &MacroNameTok, const MacroInfo *MI, + const SourceManager &SourceMgr); /// Add a usage of a macro if it already has a violation. - void expandMacro(const Token &MacroNameTok, const MacroInfo *MI); + void expandMacro(const Token &MacroNameTok, const MacroInfo *MI, + const SourceManager &SourceMgr); void addUsage(const RenamerClangTidyCheck::NamingCheckId &Decl, - SourceRange Range, const SourceManager *SourceMgr = nullptr); + SourceRange Range, const SourceManager &SourceMgr); /// Convenience method when the usage to be added is a NamedDecl. void addUsage(const NamedDecl *Decl, SourceRange Range, - const SourceManager *SourceMgr = nullptr); + const SourceManager &SourceMgr); void checkNamedDecl(const NamedDecl *Decl, const SourceManager &SourceMgr); `````````` </details> https://github.com/llvm/llvm-project/pull/88268 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
