Configure clang-tidy

CMakeLists.txt

@@ -132,6 +132,32 @@ if(CMAKE_BUILD_TYPE STREQUAL "Debug")
         clangLex
         clangSema
         clangSerialization
+        clangTidy
+        clangTidyUtils
+        # ALL_CLANG_TIDY_CHECKS
+        clangTidyAndroidModule
+        clangTidyAbseilModule
+        clangTidyAlteraModule
+        clangTidyBoostModule
+        clangTidyBugproneModule
+        clangTidyCERTModule
+        clangTidyConcurrencyModule
+        clangTidyCppCoreGuidelinesModule
+        clangTidyDarwinModule
+        clangTidyFuchsiaModule
+        clangTidyGoogleModule
+        clangTidyHICPPModule
+        clangTidyLinuxKernelModule
+        clangTidyLLVMModule
+        clangTidyLLVMLibcModule
+        clangTidyMiscModule
+        clangTidyModernizeModule
+        clangTidyObjCModule
+        clangTidyOpenMPModule
+        clangTidyPerformanceModule
+        clangTidyPortabilityModule
+        clangTidyReadabilityModule
+        clangTidyZirconModule
         clangTooling
         clangToolingCore
         clangToolingInclusions
@@ -147,6 +173,7 @@ target_compile_options(clice-core PUBLIC ${CLICE_CXX_FLAGS})
 target_link_options(clice-core PUBLIC ${CLICE_LINKER_FLAGS})
 
 target_include_directories(clice-core PUBLIC "${CMAKE_SOURCE_DIR}/include")
+target_include_directories(clice-core PRIVATE "${CMAKE_SOURCE_DIR}/src/Compiler/generated/")
 target_link_libraries(clice-core PUBLIC uv_a tomlplusplus::tomlplusplus llvm-libs)
 
 # clice executable

include/Compiler/Compilation.h

@@ -15,6 +15,9 @@ struct CompilationParams {
     /// The kind of this compilation.
     CompilationUnit::Kind kind;
 
+    /// Whether to run clang-tidy.
+    bool clang_tidy = false;
+
     /// Output file path.
     llvm::SmallString<128> output_file;
 

include/Compiler/Diagnostic.h

@@ -52,6 +52,8 @@ struct DiagnosticID {
     bool is_unused() const;
 };
 
+class DiagnosticCollector {};
+
 struct Diagnostic {
     /// The diagnostic id.
     DiagnosticID id;
@@ -66,7 +68,8 @@ struct Diagnostic {
     /// The error message of this diagnostic.
     std::string message;
 
-    static clang::DiagnosticConsumer* create(std::shared_ptr<std::vector<Diagnostic>> diagnostics);
+    static std::pair<DiagnosticCollector*, clang::DiagnosticConsumer*>
+        create(std::shared_ptr<std::vector<Diagnostic>> diagnostics);
 };
 
 }  // namespace clice

include/Compiler/Tidy.h

@@ -0,0 +1,24 @@
+#pragma once
+
+#include "llvm/ADT/StringRef.h"
+
+#include <memory>
+
+namespace clang {
+class CompilerInstance;
+}
+
+namespace clice::tidy {
+
+bool isRegisteredTidyCheck(llvm::StringRef check);
+std::optional<bool> isFastTidyCheck(llvm::StringRef check);
+
+struct TidyParams {};
+
+class ClangTidyChecker;
+
+/// Run clang-tidy on the given file.
+std::unique_ptr<ClangTidyChecker> configure(clang::CompilerInstance& instance,
+                                            const TidyParams& params);
+
+}  // namespace clice::tidy

include/Server/Config.h

@@ -5,6 +5,7 @@
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringMap.h"
 
 namespace clice::config {
 
@@ -30,6 +31,13 @@ struct IndexOptions {
     std::string dir = "${workspace}/.clice/index";
 };
 
+/// Configures what clang-tidy checks to run and options to use with them.
+struct ClangTidyOptions {
+    // A comma-separated list of globs specify which clang-tidy checks to run.
+    std::string checks;
+    llvm::StringMap<std::string> check_options;
+};
+
 struct Rule {
     std::string pattern;
     std::vector<std::string> append;
@@ -44,6 +52,7 @@ extern llvm::StringRef binary;
 extern llvm::StringRef llvm_version;
 extern llvm::StringRef workspace;
 
+extern const ClangTidyOptions& clang_tidy;
 extern const ServerOptions& server;
 extern const CacheOptions& cache;
 extern const IndexOptions& index;

src/Compiler/Compilation.cpp

@@ -1,11 +1,15 @@
+#include "Support/Logger.h"
 #include "CompilationUnitImpl.h"
 #include "Compiler/Command.h"
 #include "Compiler/Compilation.h"
 #include "Compiler/Diagnostic.h"
+#include "Compiler/Tidy.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Frontend/MultiplexConsumer.h"
 
+#include "TidyImpl.h"
+
 namespace clice {
 
 namespace {
@@ -63,6 +67,56 @@ class ProxyASTConsumer final : public clang::MultiplexConsumer {
     std::shared_ptr<std::atomic_bool> stop;
 };
 
+template <class T>
+bool isTemplateSpecializationKind(const clang::NamedDecl* D,
+                                  clang::TemplateSpecializationKind Kind) {
+    if(const auto* TD = dyn_cast<T>(D))
+        return TD->getTemplateSpecializationKind() == Kind;
+    return false;
+}
+
+bool isTemplateSpecializationKind(const clang::NamedDecl* D,
+                                  clang::TemplateSpecializationKind Kind) {
+    return isTemplateSpecializationKind<clang::FunctionDecl>(D, Kind) ||
+           isTemplateSpecializationKind<clang::CXXRecordDecl>(D, Kind) ||
+           isTemplateSpecializationKind<clang::VarDecl>(D, Kind);
+}
+
+bool isImplicitTemplateInstantiation(const clang::NamedDecl* D) {
+    return isTemplateSpecializationKind(D, clang::TSK_ImplicitInstantiation);
+}
+
+class DeclTrackingASTConsumer : public clang::ASTConsumer {
+public:
+    DeclTrackingASTConsumer(std::vector<clang::Decl*>& TopLevelDecls) :
+        TopLevelDecls(TopLevelDecls) {}
+
+    bool HandleTopLevelDecl(clang::DeclGroupRef DG) override {
+        for(clang::Decl* D: DG) {
+
+            TopLevelDecls.push_back(D);
+        }
+        return true;
+    }
+
+private:
+    std::vector<clang::Decl*>& TopLevelDecls;
+};
+
+bool isClangdTopLevelDecl(const clang::Decl* D) {
+    auto& SM = D->getASTContext().getSourceManager();
+    if(!isInsideMainFile(D->getLocation(), SM))
+        return false;
+    if(const clang::NamedDecl* ND = dyn_cast<clang::NamedDecl>(D))
+        if(isImplicitTemplateInstantiation(ND))
+            return false;
+
+    // ObjCMethodDecl are not actually top-level decls.
+    if(isa<clang::ObjCMethodDecl>(D))
+        return false;
+    return true;
+}
+
 class ProxyAction final : public clang::WrapperFrontendAction {
 public:
     ProxyAction(std::unique_ptr<clang::FrontendAction> action,
@@ -80,7 +134,6 @@ class ProxyAction final : public clang::WrapperFrontendAction {
             std::move(stop));
     }
 
-    /// Make this public.
     using clang::WrapperFrontendAction::EndSourceFile;
 
 private:
@@ -154,10 +207,11 @@ CompilationResult run_clang(CompilationParams& params,
                             const AfterExecute& after_execute = no_hook) {
     auto diagnostics =
         params.diagnostics ? params.diagnostics : std::make_shared<std::vector<Diagnostic>>();
+    auto [collector, diagnostic_client] = Diagnostic::create(diagnostics);
     auto diagnostic_engine =
         clang::CompilerInstance::createDiagnostics(*params.vfs,
                                                    new clang::DiagnosticOptions(),
-                                                   Diagnostic::create(diagnostics));
+                                                   diagnostic_client);
 
     auto invocation = create_invocation(params, diagnostic_engine);
     if(!invocation) {
@@ -189,15 +243,22 @@ CompilationResult run_clang(CompilationParams& params,
     auto action = std::make_unique<ProxyAction>(
         std::make_unique<Action>(),
         /// We only collect top level declarations for parse main file.
-        params.kind == CompilationUnit::Content ? &top_level_decls : nullptr,
+        (params.clang_tidy || params.kind == CompilationUnit::Content) ? &top_level_decls : nullptr,
         params.stop);
 
     if(!action->BeginSourceFile(*instance, instance->getFrontendOpts().Inputs[0])) {
         return std::unexpected("Fail to begin source file");
     }
 
     auto& pp = instance->getPreprocessor();
-    /// FIXME: clang-tidy, include-fixer, etc?
+    /// FIXME: include-fixer, etc?
+
+    /// Setup clang-tidy
+    std::unique_ptr<tidy::ClangTidyChecker> checker;
+    if(params.clang_tidy) {
+        tidy::TidyParams params;
+        checker = tidy::configure(*instance, params);
+    }
 
     /// `BeginSourceFile` may create new preprocessor, so all operations related to preprocessor
     /// should be done after `BeginSourceFile`.
@@ -235,6 +296,25 @@ CompilationResult run_clang(CompilationParams& params,
         token_buffer = std::move(*token_collector).consume();
     }
 
+    // Must be called before EndSourceFile because the ast context can be destroyed later.
+    if(checker) {
+        auto clangd_top_level_decls = top_level_decls;
+        std::erase_if(clangd_top_level_decls,
+                      [](auto decl) { return !isClangdTopLevelDecl(decl); });
+        log::info("Clangd top level decls: {} of {}",
+                  clangd_top_level_decls.size(),
+                  top_level_decls.size());
+        // AST traversals should exclude the preamble, to avoid performance cliffs.
+        // TODO: is it okay to affect the unit-level traversal scope here?
+        instance->getASTContext().setTraversalScope(clangd_top_level_decls);
+        checker->CTFinder.matchAST(instance->getASTContext());
+    }
+
+    // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+    // However Action->EndSourceFile() would destroy the ASTContext!
+    // So just inform the preprocessor of EOF, while keeping everything alive.
+    pp.EndSourceFile();
+
     /// FIXME: getDependencies currently return ArrayRef<std::string>, which actually results in
     /// extra copy. It would be great to avoid this copy.
 
@@ -244,7 +324,7 @@ CompilationResult run_clang(CompilationParams& params,
     }
 
     auto impl = new CompilationUnit::Impl{
-        .interested = pp.getSourceManager().getMainFileID(),
+        .interested = instance->getSourceManager().getMainFileID(),
         .src_mgr = instance->getSourceManager(),
         .action = std::move(action),
         .instance = std::move(instance),

src/Compiler/CompilationUnit.cpp

@@ -6,6 +6,11 @@ namespace clice {
 
 CompilationUnit::~CompilationUnit() {
     if(impl && impl->action) {
+        auto instance = impl->instance.get();
+        // We already notified the PP of end-of-file earlier, so detach it first.
+        // We must keep it alive until after EndSourceFile(), Sema relies on this.
+        auto PP = instance->getPreprocessorPtr();
+        instance->setPreprocessor(nullptr);  // Detach so we don't send EOF again.
         impl->action->EndSourceFile();
     }
 

src/Compiler/Diagnostic.cpp

@@ -194,9 +194,9 @@ auto diagnostic_range(const clang::Diagnostic& diagnostic, const clang::LangOpti
     };
 }
 
-class DiagnosticCollector : public clang::DiagnosticConsumer {
+class DiagnosticCollectorImpl : public clang::DiagnosticConsumer, public DiagnosticCollector {
 public:
-    DiagnosticCollector(std::shared_ptr<std::vector<Diagnostic>> diagnostics) :
+    DiagnosticCollectorImpl(std::shared_ptr<std::vector<Diagnostic>> diagnostics) :
         diagnostics(diagnostics) {}
 
     void BeginSourceFile(const clang::LangOptions& Opts, const clang::Preprocessor* PP) override {
@@ -241,9 +241,10 @@ class DiagnosticCollector : public clang::DiagnosticConsumer {
     clang::SourceManager* src_mgr;
 };
 
-clang::DiagnosticConsumer*
+std::pair<DiagnosticCollector*, clang::DiagnosticConsumer*>
     Diagnostic::create(std::shared_ptr<std::vector<Diagnostic>> diagnostics) {
-    return new DiagnosticCollector(diagnostics);
+    auto collector = new DiagnosticCollectorImpl(diagnostics);
+    return std::pair{collector, collector};
 }
 
 }  // namespace clice