[clang-tidy] Fix ClangTidyTest to initialize context before checks.

[ymandel]

Summary

Currently, clang::tidy::test::runCheckOnCode() constructs the check
instances *before* initializing the ClangTidyContext. This ordering causes
problems when the check's constructor accesses the context, for example, through
getLangOpts().

This revision moves the construction to after the context initialization, which
follows the pattern used in the clang tidy tool itself.

[hokein]

looks good.

[gribozavr]

Good catch!

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

48 ↗

(On Diff #206509)

"CheckTypes"? 'cause "Checks" below is also technically a "check list".

65 ↗

(On Diff #206509)

Please explain why ("so that check constructors can access the context")

[ymandel]

responded to comments.

[ymandel]

clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

48 ↗

(On Diff #206509)

Agreed. I was just blindly following what was already there. Changed here and below.

clang-tools-extra/trunk/unittests/clang-tidy/ClangTidyTest.h

 #include "llvm/Support/Path.h"
 #include <map>
 #include <memory>
 
 namespace clang {
 namespace tidy {
 namespace test {
 
+template <typename Check, typename... Checks> struct CheckFactory {
+  static void
+  createChecks(ClangTidyContext *Context,
+               SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
+    CheckFactory<Check>::createChecks(Context, Result);
+    CheckFactory<Checks...>::createChecks(Context, Result);
+  }
+};
+
+template <typename Check> struct CheckFactory<Check> {
+  static void
+  createChecks(ClangTidyContext *Context,
+               SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
+    Result.emplace_back(llvm::make_unique<Check>(
+        "test-check-" + std::to_string(Result.size()), Context));
+  }
+};
+
+template <typename... CheckTypes>
 class TestClangTidyAction : public ASTFrontendAction {
 public:
   TestClangTidyAction(SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Checks,
                       ast_matchers::MatchFinder &Finder,
                       ClangTidyContext &Context)
       : Checks(Checks), Finder(Finder), Context(Context) {}
 
 private:
   std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &Compiler,
                                                  StringRef File) override {
     Context.setSourceManager(&Compiler.getSourceManager());
     Context.setCurrentFile(File);
     Context.setASTContext(&Compiler.getASTContext());
 
     Preprocessor *PP = &Compiler.getPreprocessor();
+
+    // Checks must be created here, _after_ `Context` has been initialized, so
+    // that check constructors can access the context (for example, through
+    // `getLangOpts()`).
+    CheckFactory<CheckTypes...>::createChecks(&Context, Checks);
     for (auto &Check : Checks) {
       Check->registerMatchers(&Finder);
       Check->registerPPCallbacks(Compiler.getSourceManager(), PP, PP);
     }
     return Finder.newASTConsumer();
   }
 
   SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Checks;
   ast_matchers::MatchFinder &Finder;
   ClangTidyContext &Context;
 };
 
-template <typename Check, typename... Checks> struct CheckFactory {
-  static void
-  createChecks(ClangTidyContext *Context,
-               SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
-    CheckFactory<Check>::createChecks(Context, Result);
-    CheckFactory<Checks...>::createChecks(Context, Result);
-  }
-};
-
-template <typename Check> struct CheckFactory<Check> {
-  static void
-  createChecks(ClangTidyContext *Context,
-               SmallVectorImpl<std::unique_ptr<ClangTidyCheck>> &Result) {
-    Result.emplace_back(llvm::make_unique<Check>(
-        "test-check-" + std::to_string(Result.size()), Context));
-  }
-};
-
-template <typename... CheckList>
+template <typename... CheckTypes>
 std::string
 runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
                const Twine &Filename = "input.cc",
                ArrayRef<std::string> ExtraArgs = None,
                const ClangTidyOptions &ExtraOptions = ClangTidyOptions(),
                std::map<StringRef, StringRef> PathsToContent =
                    std::map<StringRef, StringRef>()) {
   ClangTidyOptions Options = ExtraOptions;
runCheckOnCode(StringRef Code, std::vector<ClangTidyError> *Errors = nullptr,
 
   ast_matchers::MatchFinder Finder;
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
       new llvm::vfs::InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr<FileManager> Files(
       new FileManager(FileSystemOptions(), InMemoryFileSystem));
 
   SmallVector<std::unique_ptr<ClangTidyCheck>, 1> Checks;
-  CheckFactory<CheckList...>::createChecks(&Context, Checks);
   tooling::ToolInvocation Invocation(
-      Args, new TestClangTidyAction(Checks, Finder, Context), Files.get());
+      Args, new TestClangTidyAction<CheckTypes...>(Checks, Finder, Context),
+      Files.get());
   InMemoryFileSystem->addFile(Filename, 0,
                               llvm::MemoryBuffer::getMemBuffer(Code));
   for (const auto &FileContent : PathsToContent) {
     InMemoryFileSystem->addFile(
         Twine("include/") + FileContent.first, 0,
         llvm::MemoryBuffer::getMemBuffer(FileContent.second));
   }
   Invocation.setDiagnosticConsumer(&DiagConsumer);