[clang-tidy] Refactor IncludeInserter
authorNathan James <n.james93@hotmail.co.uk>
Mon, 27 Jul 2020 11:48:53 +0000 (12:48 +0100)
committerNathan James <n.james93@hotmail.co.uk>
Mon, 27 Jul 2020 11:48:55 +0000 (12:48 +0100)
Simplified how `IncludeInserter` is used in Checks by abstracting away the SourceManager and PPCallbacks inside the method `registerPreprocessor`.
Changed checks that use `IncludeInserter` to no longer use a `std::unique_ptr`, instead the IncludeInserter is just a member of the class thats initialized with an `IncludeStyle`.
Saving an unnecessary allocation.

This results in the removal of the field `IncludeSorter::IncludeStyle` from the checks, as its wrapped in the `IncludeInserter`.
No longer need to create an instance of the `IncludeInserter` in the registerPPCallbacks, now that method only needs to contain:
```
Inserter.registerPreprocessor(PP);
```
Also added a helper method to `IncludeInserter` called `createMainFileInclusionInsertion`, purely sugar but does better express intentions.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D83680

25 files changed:
clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.cpp
clang-tools-extra/clang-tidy/abseil/StringFindStartswithCheck.h
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.h
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.h
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.h
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.h
clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceAutoPtrCheck.h
clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp
clang-tools-extra/clang-tidy/modernize/ReplaceRandomShuffleCheck.h
clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.cpp
clang-tools-extra/clang-tidy/performance/MoveConstructorInitCheck.h
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.cpp
clang-tools-extra/clang-tidy/performance/TypePromotionInMathFnCheck.h
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tools-extra/clang-tidy/utils/IncludeInserter.cpp
clang-tools-extra/clang-tidy/utils/IncludeInserter.h
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.cpp
clang-tools-extra/clang-tidy/utils/TransformerClangTidyCheck.h
clang-tools-extra/unittests/clang-tidy/IncludeInserterTest.cpp

index 11bbcbc..e775fc2 100644 (file)
@@ -26,8 +26,8 @@ StringFindStartswithCheck::StringFindStartswithCheck(StringRef Name,
     : ClangTidyCheck(Name, Context),
       StringLikeClasses(utils::options::parseStringList(
           Options.get("StringLikeClasses", "::std::basic_string"))),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)),
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM)),
       AbseilStringsMatchHeader(
           Options.get("AbseilStringsMatchHeader", "absl/strings/match.h")) {}
 
@@ -105,23 +105,21 @@ void StringFindStartswithCheck::check(const MatchFinder::MatchResult &Result) {
 
   // Create a preprocessor #include FixIt hint (CreateIncludeInsertion checks
   // whether this already exists).
-  Diagnostic << IncludeInserter->CreateIncludeInsertion(
+  Diagnostic << IncludeInserter.createIncludeInsertion(
       Source.getFileID(ComparisonExpr->getBeginLoc()), AbseilStringsMatchHeader,
       false);
 }
 
 void StringFindStartswithCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  IncludeInserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                              IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
+  IncludeInserter.registerPreprocessor(PP);
 }
 
 void StringFindStartswithCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "StringLikeClasses",
                 utils::options::serializeStringList(StringLikeClasses));
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
   Options.store(Opts, "AbseilStringsMatchHeader", AbseilStringsMatchHeader);
 }
 
index d232d3b..2bb20f7 100644 (file)
@@ -35,9 +35,8 @@ public:
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
-  std::unique_ptr<clang::tidy::utils::IncludeInserter> IncludeInserter;
   const std::vector<std::string> StringLikeClasses;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter IncludeInserter;
   const std::string AbseilStringsMatchHeader;
 };
 
index f1755d3..3f51ef5 100644 (file)
@@ -26,12 +26,12 @@ AST_MATCHER(VarDecl, isLocalVarDecl) { return Node.isLocalVarDecl(); }
 InitVariablesCheck::InitVariablesCheck(StringRef Name,
                                        ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)),
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM)),
       MathHeader(Options.get("MathHeader", "math.h")) {}
 
 void InitVariablesCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
   Options.store(Opts, "MathHeader", MathHeader);
 }
 
@@ -51,9 +51,7 @@ void InitVariablesCheck::registerMatchers(MatchFinder *Finder) {
 void InitVariablesCheck::registerPPCallbacks(const SourceManager &SM,
                                              Preprocessor *PP,
                                              Preprocessor *ModuleExpanderPP) {
-  IncludeInserter =
-      std::make_unique<utils::IncludeInserter>(SM, getLangOpts(), IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
+  IncludeInserter.registerPreprocessor(PP);
 }
 
 void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
@@ -104,7 +102,7 @@ void InitVariablesCheck::check(const MatchFinder::MatchResult &Result) {
                    MatchedDecl->getName().size()),
                InitializationString);
     if (AddMathInclude) {
-      Diagnostic << IncludeInserter->CreateIncludeInsertion(
+      Diagnostic << IncludeInserter.createIncludeInsertion(
           Source.getFileID(MatchedDecl->getBeginLoc()), MathHeader, false);
     }
   }
index 61521b1..0f77810 100644 (file)
@@ -31,8 +31,7 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unique_ptr<clang::tidy::utils::IncludeInserter> IncludeInserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter IncludeInserter;
   const std::string MathHeader;
 };
 
index 96b0bb0..f45801f 100644 (file)
@@ -21,20 +21,18 @@ namespace cppcoreguidelines {
 ProBoundsConstantArrayIndexCheck::ProBoundsConstantArrayIndexCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), GslHeader(Options.get("GslHeader", "")),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)) {}
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)) {}
 
 void ProBoundsConstantArrayIndexCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "GslHeader", GslHeader);
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 void ProBoundsConstantArrayIndexCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                       IncludeStyle);
-  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void ProBoundsConstantArrayIndexCheck::registerMatchers(MatchFinder *Finder) {
@@ -87,9 +85,8 @@ void ProBoundsConstantArrayIndexCheck::check(
                               IndexRange.getBegin().getLocWithOffset(-1)),
                   ", ")
            << FixItHint::CreateReplacement(Matched->getEndLoc(), ")")
-           << Inserter->CreateIncludeInsertion(
-                  Result.SourceManager->getMainFileID(), GslHeader,
-                  /*IsAngled=*/false);
+           << Inserter.createMainFileIncludeInsertion(GslHeader,
+                                                      /*IsAngled=*/false);
     }
     return;
   }
index ac7475b..04a51b9 100644 (file)
@@ -23,8 +23,7 @@ namespace cppcoreguidelines {
 /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.html
 class ProBoundsConstantArrayIndexCheck : public ClangTidyCheck {
   const std::string GslHeader;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
-  std::unique_ptr<utils::IncludeInserter> Inserter;
+  utils::IncludeInserter Inserter;
 
 public:
   ProBoundsConstantArrayIndexCheck(StringRef Name, ClangTidyContext *Context);
index c677043..5818b8c 100644 (file)
@@ -44,8 +44,8 @@ const char MakeSmartPtrCheck::PointerType[] = "pointerType";
 MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
                                      StringRef MakeSmartPtrFunctionName)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)),
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)),
       MakeSmartPtrFunctionHeader(
           Options.get("MakeSmartPtrFunctionHeader", StdMemoryHeader)),
       MakeSmartPtrFunctionName(
@@ -53,7 +53,7 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name, ClangTidyContext *Context,
       IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "MakeSmartPtrFunctionHeader", MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
@@ -67,9 +67,7 @@ bool MakeSmartPtrCheck::isLanguageVersionSupported(
 void MakeSmartPtrCheck::registerPPCallbacks(const SourceManager &SM,
                                             Preprocessor *PP,
                                             Preprocessor *ModuleExpanderPP) {
-    Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                         IncludeStyle);
-    PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void MakeSmartPtrCheck::registerMatchers(ast_matchers::MatchFinder *Finder) {
@@ -432,7 +430,7 @@ void MakeSmartPtrCheck::insertHeader(DiagnosticBuilder &Diag, FileID FD) {
   if (MakeSmartPtrFunctionHeader.empty()) {
     return;
   }
-  Diag << Inserter->CreateIncludeInsertion(
+  Diag << Inserter.createIncludeInsertion(
       FD, MakeSmartPtrFunctionHeader,
       /*IsAngled=*/MakeSmartPtrFunctionHeader == StdMemoryHeader);
 }
index 1f73873..7a1bba6 100644 (file)
@@ -46,8 +46,7 @@ protected:
   static const char PointerType[];
 
 private:
-  std::unique_ptr<utils::IncludeInserter> Inserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter Inserter;
   const std::string MakeSmartPtrFunctionHeader;
   const std::string MakeSmartPtrFunctionName;
   const bool IgnoreMacros;
index b6dedfb..b955ea7 100644 (file)
@@ -120,12 +120,12 @@ collectParamDecls(const CXXConstructorDecl *Ctor,
 
 PassByValueCheck::PassByValueCheck(StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)),
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)),
       ValuesOnly(Options.get("ValuesOnly", false)) {}
 
 void PassByValueCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "ValuesOnly", ValuesOnly);
 }
 
@@ -167,9 +167,7 @@ void PassByValueCheck::registerMatchers(MatchFinder *Finder) {
 void PassByValueCheck::registerPPCallbacks(const SourceManager &SM,
                                            Preprocessor *PP,
                                            Preprocessor *ModuleExpanderPP) {
-    Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                         IncludeStyle);
-    PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
@@ -216,7 +214,7 @@ void PassByValueCheck::check(const MatchFinder::MatchResult &Result) {
   Diag << FixItHint::CreateInsertion(Initializer->getRParenLoc(), ")")
        << FixItHint::CreateInsertion(
               Initializer->getLParenLoc().getLocWithOffset(1), "std::move(")
-       << Inserter->CreateIncludeInsertion(
+       << Inserter.createIncludeInsertion(
               Result.SourceManager->getFileID(Initializer->getSourceLocation()),
               "utility",
               /*IsAngled=*/true);
index 7abda91..82cd9d4 100644 (file)
@@ -31,8 +31,7 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unique_ptr<utils::IncludeInserter> Inserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter Inserter;
   const bool ValuesOnly;
 };
 
index f98254d..25ffbe2 100644 (file)
@@ -74,11 +74,11 @@ AST_MATCHER(Decl, isFromStdNamespace) {
 ReplaceAutoPtrCheck::ReplaceAutoPtrCheck(StringRef Name,
                                          ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)) {}
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)) {}
 
 void ReplaceAutoPtrCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 void ReplaceAutoPtrCheck::registerMatchers(MatchFinder *Finder) {
@@ -131,9 +131,7 @@ void ReplaceAutoPtrCheck::registerMatchers(MatchFinder *Finder) {
 void ReplaceAutoPtrCheck::registerPPCallbacks(const SourceManager &SM,
                                               Preprocessor *PP,
                                               Preprocessor *ModuleExpanderPP) {
-  Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                       IncludeStyle);
-  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void ReplaceAutoPtrCheck::check(const MatchFinder::MatchResult &Result) {
@@ -146,12 +144,11 @@ void ReplaceAutoPtrCheck::check(const MatchFinder::MatchResult &Result) {
     if (Range.isInvalid())
       return;
 
-    auto Diag =
-        diag(Range.getBegin(), "use std::move to transfer ownership")
-        << FixItHint::CreateInsertion(Range.getBegin(), "std::move(")
-        << FixItHint::CreateInsertion(Range.getEnd(), ")")
-        << Inserter->CreateIncludeInsertion(SM.getMainFileID(), "utility",
-                                            /*IsAngled=*/true);
+    auto Diag = diag(Range.getBegin(), "use std::move to transfer ownership")
+                << FixItHint::CreateInsertion(Range.getBegin(), "std::move(")
+                << FixItHint::CreateInsertion(Range.getEnd(), ")")
+                << Inserter.createMainFileIncludeInsertion("utility",
+                                                           /*IsAngled=*/true);
 
     return;
   }
index e2b0407..8288c7e 100644 (file)
@@ -53,8 +53,7 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unique_ptr<utils::IncludeInserter> Inserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter Inserter;
 };
 
 } // namespace modernize
index 66917df..0191f5d 100644 (file)
@@ -23,8 +23,9 @@ namespace modernize {
 ReplaceRandomShuffleCheck::ReplaceRandomShuffleCheck(StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)) {}
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM)) {
+}
 
 void ReplaceRandomShuffleCheck::registerMatchers(MatchFinder *Finder) {
   const auto Begin = hasArgument(0, expr());
@@ -44,14 +45,12 @@ void ReplaceRandomShuffleCheck::registerMatchers(MatchFinder *Finder) {
 
 void ReplaceRandomShuffleCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  IncludeInserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                              IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
+  IncludeInserter.registerPreprocessor(PP);
 }
 
 void ReplaceRandomShuffleCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
 }
 
 void ReplaceRandomShuffleCheck::check(const MatchFinder::MatchResult &Result) {
@@ -92,7 +91,7 @@ void ReplaceRandomShuffleCheck::check(const MatchFinder::MatchResult &Result) {
 
   Diag << FixItHint::CreateRemoval(MatchedDecl->getSourceRange());
   Diag << FixItHint::CreateInsertion(MatchedDecl->getBeginLoc(), NewName);
-  Diag << IncludeInserter->CreateIncludeInsertion(
+  Diag << IncludeInserter.createIncludeInsertion(
       Result.Context->getSourceManager().getFileID(
           MatchedCallExpr->getBeginLoc()),
       "random", /*IsAngled=*/true);
index c4ac74d..990dcff 100644 (file)
@@ -34,8 +34,7 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unique_ptr<utils::IncludeInserter> IncludeInserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter IncludeInserter;
 };
 
 } // namespace modernize
index 4cbb014..6b42cd3 100644 (file)
@@ -23,8 +23,8 @@ namespace performance {
 MoveConstructorInitCheck::MoveConstructorInitCheck(StringRef Name,
                                                    ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)) {}
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)) {}
 
 void MoveConstructorInitCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
@@ -90,13 +90,11 @@ void MoveConstructorInitCheck::check(const MatchFinder::MatchResult &Result) {
 
 void MoveConstructorInitCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                       IncludeStyle);
-  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void MoveConstructorInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 } // namespace performance
index 0473978..0b637b6 100644 (file)
@@ -36,8 +36,7 @@ public:
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
 
 private:
-  std::unique_ptr<utils::IncludeInserter> Inserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter Inserter;
 };
 
 } // namespace performance
index 597445d..2105aa9 100644 (file)
@@ -31,19 +31,18 @@ AST_MATCHER_P(Type, isBuiltinType, BuiltinType::Kind, Kind) {
 TypePromotionInMathFnCheck::TypePromotionInMathFnCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)) {}
+      IncludeInserter(Options.getLocalOrGlobal("IncludeStyle",
+                                               utils::IncludeSorter::IS_LLVM)) {
+}
 
 void TypePromotionInMathFnCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  IncludeInserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                              IncludeStyle);
-  PP->addPPCallbacks(IncludeInserter->CreatePPCallbacks());
+  IncludeInserter.registerPreprocessor(PP);
 }
 
 void TypePromotionInMathFnCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", IncludeInserter.getStyle());
 }
 
 void TypePromotionInMathFnCheck::registerMatchers(MatchFinder *Finder) {
@@ -191,7 +190,7 @@ void TypePromotionInMathFnCheck::check(const MatchFinder::MatchResult &Result) {
   // <math.h>, because the functions we're suggesting moving away from are all
   // declared in <math.h>.
   if (FnInCmath)
-    Diag << IncludeInserter->CreateIncludeInsertion(
+    Diag << IncludeInserter.createIncludeInsertion(
         Result.Context->getSourceManager().getFileID(Call->getBeginLoc()),
         "cmath", /*IsAngled=*/true);
 }
index d1cc042..dd7c1c0 100644 (file)
@@ -36,8 +36,7 @@ public:
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
 
 private:
-  std::unique_ptr<utils::IncludeInserter> IncludeInserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter IncludeInserter;
 };
 
 } // namespace performance
index 5de53b1..9aef5a8 100644 (file)
@@ -68,8 +68,8 @@ bool isExplicitTemplateSpecialization(const FunctionDecl &Function) {
 UnnecessaryValueParamCheck::UnnecessaryValueParamCheck(
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            utils::IncludeSorter::IS_LLVM)),
+      Inserter(Options.getLocalOrGlobal("IncludeStyle",
+                                        utils::IncludeSorter::IS_LLVM)),
       AllowedTypes(
           utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 
@@ -173,14 +173,12 @@ void UnnecessaryValueParamCheck::check(const MatchFinder::MatchResult &Result) {
 
 void UnnecessaryValueParamCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  Inserter = std::make_unique<utils::IncludeInserter>(SM, getLangOpts(),
-                                                      IncludeStyle);
-  PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+  Inserter.registerPreprocessor(PP);
 }
 
 void UnnecessaryValueParamCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
   Options.store(Opts, "AllowedTypes",
                 utils::options::serializeStringList(AllowedTypes));
 }
@@ -204,7 +202,7 @@ void UnnecessaryValueParamCheck::handleMoveFix(const ParmVarDecl &Var,
                                            Context.getLangOpts());
   Diag << FixItHint::CreateInsertion(CopyArgument.getBeginLoc(), "std::move(")
        << FixItHint::CreateInsertion(EndLoc, ")")
-       << Inserter->CreateIncludeInsertion(
+       << Inserter.createIncludeInsertion(
               SM.getFileID(CopyArgument.getBeginLoc()), "utility",
               /*IsAngled=*/true);
 }
index 1d23671..a84079e 100644 (file)
@@ -41,8 +41,7 @@ private:
 
   llvm::DenseMap<const FunctionDecl *, FunctionParmMutationAnalyzer>
       MutationAnalyzers;
-  std::unique_ptr<utils::IncludeInserter> Inserter;
-  const utils::IncludeSorter::IncludeStyle IncludeStyle;
+  utils::IncludeInserter Inserter;
   const std::vector<std::string> AllowedTypes;
 };
 
index df87dbe..268692c 100644 (file)
@@ -7,6 +7,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "IncludeInserter.h"
+#include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
 
 namespace clang {
@@ -26,7 +28,7 @@ public:
                           StringRef /*SearchPath*/, StringRef /*RelativePath*/,
                           const Module * /*ImportedModule*/,
                           SrcMgr::CharacteristicKind /*FileType*/) override {
-    Inserter->AddInclude(FileNameRef, IsAngled, HashLocation,
+    Inserter->addInclude(FileNameRef, IsAngled, HashLocation,
                          IncludeToken.getEndLoc());
   }
 
@@ -34,45 +36,61 @@ private:
   IncludeInserter *Inserter;
 };
 
-IncludeInserter::IncludeInserter(const SourceManager &SourceMgr,
-                                 const LangOptions &LangOpts,
-                                 IncludeSorter::IncludeStyle Style)
-    : SourceMgr(SourceMgr), Style(Style) {}
+IncludeInserter::IncludeInserter(IncludeSorter::IncludeStyle Style)
+    : Style(Style) {}
 
-IncludeInserter::~IncludeInserter() {}
+void IncludeInserter::registerPreprocessor(Preprocessor *PP) {
+  assert(PP && "PP shouldn't be null");
+  SourceMgr = &PP->getSourceManager();
 
-std::unique_ptr<PPCallbacks> IncludeInserter::CreatePPCallbacks() {
-  return std::make_unique<IncludeInserterCallback>(this);
+  // If this gets registered multiple times, clear the maps
+  if (!IncludeSorterByFile.empty())
+    IncludeSorterByFile.clear();
+  if (!InsertedHeaders.empty())
+    InsertedHeaders.clear();
+  PP->addPPCallbacks(std::make_unique<IncludeInserterCallback>(this));
 }
 
 IncludeSorter &IncludeInserter::getOrCreate(FileID FileID) {
+  assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call "
+                      "registerPreprocessor()?");
   // std::unique_ptr is cheap to construct, so force a construction now to save
   // the lookup needed if we were to insert into the map.
   std::unique_ptr<IncludeSorter> &Entry = IncludeSorterByFile[FileID];
   if (!Entry) {
     // If it wasn't found, Entry will be default constructed to nullptr.
     Entry = std::make_unique<IncludeSorter>(
-        &SourceMgr, FileID,
-        SourceMgr.getFilename(SourceMgr.getLocForStartOfFile(FileID)), Style);
+        SourceMgr, FileID,
+        SourceMgr->getFilename(SourceMgr->getLocForStartOfFile(FileID)), Style);
   }
   return *Entry;
 }
 
 llvm::Optional<FixItHint>
-IncludeInserter::CreateIncludeInsertion(FileID FileID, StringRef Header,
+IncludeInserter::createIncludeInsertion(FileID FileID, StringRef Header,
                                         bool IsAngled) {
   // We assume the same Header will never be included both angled and not
   // angled.
-  if (!InsertedHeaders[FileID].insert(std::string(Header)).second)
+  if (!InsertedHeaders[FileID].insert(Header).second)
     return llvm::None;
 
   return getOrCreate(FileID).CreateIncludeInsertion(Header, IsAngled);
 }
 
-void IncludeInserter::AddInclude(StringRef FileName, bool IsAngled,
+llvm::Optional<FixItHint>
+IncludeInserter::createMainFileIncludeInsertion(StringRef Header,
+                                                bool IsAngled) {
+  assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call "
+                      "registerPreprocessor()?");
+  return createIncludeInsertion(SourceMgr->getMainFileID(), Header, IsAngled);
+}
+
+void IncludeInserter::addInclude(StringRef FileName, bool IsAngled,
                                  SourceLocation HashLocation,
                                  SourceLocation EndLocation) {
-  FileID FileID = SourceMgr.getFileID(HashLocation);
+  assert(SourceMgr && "SourceMgr shouldn't be null; did you remember to call "
+                      "registerPreprocessor()?");
+  FileID FileID = SourceMgr->getFileID(HashLocation);
   getOrCreate(FileID).AddInclude(FileName, IsAngled, HashLocation, EndLocation);
 }
 
index 0d4b951..70c36ce 100644 (file)
 
 #include "IncludeSorter.h"
 #include "clang/Basic/Diagnostic.h"
-#include "clang/Basic/LangOptions.h"
-#include "clang/Basic/SourceManager.h"
-#include "clang/Lex/PPCallbacks.h"
+#include "llvm/ADT/StringSet.h"
 #include <memory>
-#include <string>
 
 namespace clang {
+class Preprocessor;
 namespace tidy {
 namespace utils {
 
@@ -26,16 +24,17 @@ namespace utils {
 ///
 /// ``IncludeInserter`` can be used in clang-tidy checks in the following way:
 /// \code
+/// #include "../ClangTidyCheck.h"
 /// #include "../utils/IncludeInserter.h"
-/// #include "clang/Frontend/CompilerInstance.h"
+///
+/// namespace clang {
+/// namespace tidy {
 ///
 /// class MyCheck : public ClangTidyCheck {
 ///  public:
 ///   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
 ///                            Preprocessor *ModuleExpanderPP) override {
-///     Inserter = std::make_unique<IncludeInserter>(
-///         SM, getLangOpts(), utils::IncludeSorter::IS_Google);
-///     PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+///     Inserter.registerPreprocessor();
 ///   }
 ///
 ///   void registerMatchers(ast_matchers::MatchFinder* Finder) override { ... }
@@ -43,39 +42,53 @@ namespace utils {
 ///   void check(
 ///       const ast_matchers::MatchFinder::MatchResult& Result) override {
 ///     ...
-///     Inserter->CreateIncludeInsertion(
-///         Result.SourceManager->getMainFileID(), "path/to/Header.h",
-///         /*IsAngled=*/false);
+///     Inserter.createMainFileIncludeInsertion("path/to/Header.h",
+///                                             /*IsAngled=*/false);
 ///     ...
 ///   }
 ///
 ///  private:
-///   std::unique_ptr<clang::tidy::utils::IncludeInserter> Inserter;
+///   utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
 /// };
+/// } // namespace tidy
+/// } // namespace clang
 /// \endcode
 class IncludeInserter {
 public:
-  IncludeInserter(const SourceManager &SourceMgr, const LangOptions &LangOpts,
-                  IncludeSorter::IncludeStyle Style);
-  ~IncludeInserter();
+  /// Initializes the IncludeInserter using the IncludeStyle \p Style.
+  /// In most cases the \p Style will be retrieved from the ClangTidyOptions
+  /// using \code
+  ///   Options.getLocalOrGlobal("IncludeStyle", <DefaultStyle>)
+  /// \endcode
+  explicit IncludeInserter(IncludeSorter::IncludeStyle Style);
+
+  /// Registers this with the Preprocessor \p PP, must be called before this
+  /// class is used.
+  void registerPreprocessor(Preprocessor *PP);
 
-  /// Create ``PPCallbacks`` for registration with the compiler's preprocessor.
-  std::unique_ptr<PPCallbacks> CreatePPCallbacks();
+  /// Creates a \p Header inclusion directive fixit in the File \p FileID.
+  /// Returns ``llvm::None`` on error or if the inclusion directive already
+  /// exists.
+  llvm::Optional<FixItHint>
+  createIncludeInsertion(FileID FileID, llvm::StringRef Header, bool IsAngled);
 
-  /// Creates a \p Header inclusion directive fixit. Returns ``llvm::None`` on
-  /// error or if inclusion directive already exists.
+  /// Creates a \p Header inclusion directive fixit in the main file.
+  /// Returns``llvm::None`` on error or if the inclusion directive already
+  /// exists.
   llvm::Optional<FixItHint>
-  CreateIncludeInsertion(FileID FileID, llvm::StringRef Header, bool IsAngled);
+  createMainFileIncludeInsertion(llvm::StringRef Header, bool IsAngled);
+
+  IncludeSorter::IncludeStyle getStyle() const { return Style; }
 
 private:
-  void AddInclude(StringRef FileName, bool IsAngled,
+  void addInclude(StringRef FileName, bool IsAngled,
                   SourceLocation HashLocation, SourceLocation EndLocation);
 
   IncludeSorter &getOrCreate(FileID FileID);
 
   llvm::DenseMap<FileID, std::unique_ptr<IncludeSorter>> IncludeSorterByFile;
-  llvm::DenseMap<FileID, std::set<std::string>> InsertedHeaders;
-  const SourceManager &SourceMgr;
+  llvm::DenseMap<FileID, llvm::StringSet<>> InsertedHeaders;
+  const SourceManager *SourceMgr{nullptr};
   const IncludeSorter::IncludeStyle Style;
   friend class IncludeInserterCallback;
 };
index 03af5dd..2c116b2 100644 (file)
@@ -32,8 +32,8 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(
         MakeRule,
     StringRef Name, ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), Rule(MakeRule(getLangOpts(), Options)),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            IncludeSorter::IS_LLVM)) {
+      Inserter(
+          Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
   if (Rule)
     assert(llvm::all_of(Rule->Cases, hasExplanation) &&
            "clang-tidy checks must have an explanation by default;"
@@ -44,8 +44,8 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
                                                      StringRef Name,
                                                      ClangTidyContext *Context)
     : ClangTidyCheck(Name, Context), Rule(std::move(R)),
-      IncludeStyle(Options.getLocalOrGlobal("IncludeStyle",
-                                            IncludeSorter::IS_LLVM)) {
+      Inserter(
+          Options.getLocalOrGlobal("IncludeStyle", IncludeSorter::IS_LLVM)) {
   assert(llvm::all_of(Rule->Cases, hasExplanation) &&
          "clang-tidy checks must have an explanation by default;"
          " explicitly provide an empty explanation if none is desired");
@@ -53,15 +53,12 @@ TransformerClangTidyCheck::TransformerClangTidyCheck(RewriteRule R,
 
 void TransformerClangTidyCheck::registerPPCallbacks(
     const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) {
-  // Only allocate and register the IncludeInsert when some `Case` will add
+  // Only register the IncludeInsert when some `Case` will add
   // includes.
   if (Rule && llvm::any_of(Rule->Cases, [](const RewriteRule::Case &C) {
         return !C.AddedIncludes.empty();
-      })) {
-    Inserter =
-        std::make_unique<IncludeInserter>(SM, getLangOpts(), IncludeStyle);
-    PP->addPPCallbacks(Inserter->CreatePPCallbacks());
-  }
+      }))
+    Inserter.registerPreprocessor(PP);
 }
 
 void TransformerClangTidyCheck::registerMatchers(
@@ -102,15 +99,15 @@ void TransformerClangTidyCheck::check(
     Diag << FixItHint::CreateReplacement(T.Range, T.Replacement);
 
   for (const auto &I : Case.AddedIncludes) {
-    Diag << Inserter->CreateIncludeInsertion(
-        Result.SourceManager->getMainFileID(), I.first,
+    Diag << Inserter.createMainFileIncludeInsertion(
+        I.first,
         /*IsAngled=*/I.second == transformer::IncludeFormat::Angled);
   }
 }
 
 void TransformerClangTidyCheck::storeOptions(
     ClangTidyOptions::OptionMap &Opts) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", Inserter.getStyle());
 }
 
 } // namespace utils
index 829a22f..404f474 100644 (file)
@@ -70,8 +70,7 @@ public:
 
 private:
   Optional<transformer::RewriteRule> Rule;
-  const IncludeSorter::IncludeStyle IncludeStyle;
-  std::unique_ptr<IncludeInserter> Inserter;
+  IncludeInserter Inserter;
 };
 
 } // namespace utils
index ed5f025..e70d3fb 100644 (file)
@@ -33,9 +33,7 @@ public:
 
   void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
                            Preprocessor *ModuleExpanderPP) override {
-    Inserter = std::make_unique<utils::IncludeInserter>(
-        SM, getLangOpts(), utils::IncludeSorter::IS_Google);
-    PP->addPPCallbacks(Inserter->CreatePPCallbacks());
+    Inserter.registerPreprocessor(PP);
   }
 
   void registerMatchers(ast_matchers::MatchFinder *Finder) override {
@@ -46,15 +44,15 @@ public:
     auto Diag = diag(Result.Nodes.getNodeAs<DeclStmt>("stmt")->getBeginLoc(),
                      "foo, bar");
     for (StringRef Header : HeadersToInclude()) {
-      Diag << Inserter->CreateIncludeInsertion(
-          Result.SourceManager->getMainFileID(), Header, IsAngledInclude());
+      Diag << Inserter.createMainFileIncludeInsertion(Header,
+                                                      IsAngledInclude());
     }
   }
 
   virtual std::vector<StringRef> HeadersToInclude() const = 0;
   virtual bool IsAngledInclude() const = 0;
 
-  std::unique_ptr<utils::IncludeInserter> Inserter;
+  utils::IncludeInserter Inserter{utils::IncludeSorter::IS_Google};
 };
 
 class NonSystemHeaderInserterCheck : public IncludeInserterCheckBase {