feat: run clang-tidy

CMakeLists.txt

@@ -69,6 +69,21 @@ add_custom_target(
     DEPENDS ${GENERATED_HEADER}
 )
 
+set(CONFIG_SOURCE_FILE "${CMAKE_CURRENT_SOURCE_DIR}/config/clang-tidy-config.h")
+set(CONFIG_GENERATED_FILE "${CMAKE_CURRENT_BINARY_DIR}/generated/clang-tidy-config.h")
+
+add_custom_command(
+    OUTPUT ${CONFIG_GENERATED_FILE}
+    COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CONFIG_SOURCE_FILE} ${CONFIG_GENERATED_FILE}
+    DEPENDS ${CONFIG_SOURCE_FILE}
+    COMMENT "Generating C++ header from ${CONFIG_SOURCE_FILE}"
+)
+
+add_custom_target(
+    generate_config
+    DEPENDS ${CONFIG_GENERATED_FILE}
+)
+
 file(GLOB_RECURSE CLICE_SOURCES CONFIGURE_DEPENDS
     "${PROJECT_SOURCE_DIR}/src/AST/*.cpp"
     "${PROJECT_SOURCE_DIR}/src/Async/*.cpp"
@@ -80,7 +95,7 @@ file(GLOB_RECURSE CLICE_SOURCES CONFIGURE_DEPENDS
     "${PROJECT_SOURCE_DIR}/src/Support/*.cpp"
 )
 add_library(clice-core STATIC "${CLICE_SOURCES}")
-add_dependencies(clice-core generate_flatbuffers_schema)
+add_dependencies(clice-core generate_flatbuffers_schema generate_config)
 
 target_include_directories(clice-core PUBLIC
     "${PROJECT_SOURCE_DIR}/include"

config/clang-tidy-config.h

@@ -0,0 +1,11 @@
+/* This generated file is for internal use. Do not include it from headers. */
+
+#ifdef CLANG_TIDY_CONFIG_H
+#error clang-tidy-config.h can only be included once
+#else
+#define CLANG_TIDY_CONFIG_H
+
+// Clice currently doesn't support this configuration, and we use the same default value as clangd.
+#define CLANG_TIDY_ENABLE_STATIC_ANALYZER 0
+
+#endif

include/AST/Utility.h

@@ -15,6 +15,12 @@ bool is_templated(const clang::Decl* decl);
 /// Check whether the decl is anonymous.
 bool is_anonymous(const clang::NamedDecl* decl);
 
+/// Checks whether the location is inside the main file.
+bool is_inside_main_file(clang::SourceLocation loc, const clang::SourceManager& sm);
+
+/// Checks whether the decl is an implicit template instantiation.
+bool is_implicit_template_instantiation(const clang::NamedDecl* decl);
+
 /// Return the decl where it is instantiated from. If could be a template decl
 /// or a member of a class template. If the decl is a full specialization, return
 /// itself.

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

@@ -2,7 +2,11 @@
 
 #include <cstdint>
 #include <string>
+
 #include "AST/SourceCode.h"
+#include "Compiler/Tidy.h"
+
+#include "clang/Basic/Diagnostic.h"
 
 namespace clang {
 class DiagnosticConsumer;
@@ -52,6 +56,11 @@ struct DiagnosticID {
     bool is_unused() const;
 };
 
+class DiagnosticCollector : public clang::DiagnosticConsumer {
+public:
+    tidy::ClangTidyChecker* checker = nullptr;
+};
+
 struct Diagnostic {
     /// The diagnostic id.
     DiagnosticID id;
@@ -66,7 +75,7 @@ struct Diagnostic {
     /// The error message of this diagnostic.
     std::string message;
 
-    static clang::DiagnosticConsumer* create(std::shared_ptr<std::vector<Diagnostic>> diagnostics);
+    static DiagnosticCollector* create(std::shared_ptr<std::vector<Diagnostic>> diagnostics);
 };
 
 }  // namespace clice

include/Compiler/Tidy.h

@@ -2,9 +2,23 @@
 
 #include "llvm/ADT/StringRef.h"
 
+#include <memory>
+
+namespace clang {
+class CompilerInstance;
+}
+
 namespace clice::tidy {
 
 bool is_registered_tidy_check(llvm::StringRef check);
 std::optional<bool> is_fast_tidy_check(llvm::StringRef check);
 
+struct TidyParams {};
+
+class ClangTidyChecker;
+
+/// Configure to run clang-tidy on the given file.
+std::unique_ptr<ClangTidyChecker> configure(clang::CompilerInstance& instance,
+                                            const TidyParams& params);
+
 }  // namespace clice::tidy

src/AST/Utility.cpp

@@ -16,6 +16,13 @@
 
 namespace clice::ast {
 
+bool is_inside_main_file(clang::SourceLocation loc, const clang::SourceManager& sm) {
+    if(!loc.isValid())
+        return false;
+    clang::FileID fid = sm.getFileID(sm.getExpansionLoc(loc));
+    return fid == sm.getMainFileID() || fid == sm.getPreambleFileID();
+};
+
 bool is_definition(const clang::Decl* decl) {
     if(auto VD = llvm::dyn_cast<clang::VarDecl>(decl)) {
         return VD->isThisDeclarationADefinition();
@@ -58,6 +65,25 @@ bool is_anonymous(const clang::NamedDecl* decl) {
     return name.isIdentifier() && !name.getAsIdentifierInfo();
 }
 
+template <class T>
+bool is_template_specialization_kind(const clang::NamedDecl* decl,
+                                     clang::TemplateSpecializationKind kind) {
+    if(const auto* td = dyn_cast<T>(decl))
+        return td->getTemplateSpecializationKind() == kind;
+    return false;
+}
+
+inline bool is_template_specialization_kind(const clang::NamedDecl* decl,
+                                            clang::TemplateSpecializationKind kind) {
+    return is_template_specialization_kind<clang::FunctionDecl>(decl, kind) ||
+           is_template_specialization_kind<clang::CXXRecordDecl>(decl, kind) ||
+           is_template_specialization_kind<clang::VarDecl>(decl, kind);
+}
+
+bool is_implicit_template_instantiation(const clang::NamedDecl* decl) {
+    return is_template_specialization_kind(decl, clang::TSK_ImplicitInstantiation);
+}
+
 const static clang::CXXRecordDecl* getDeclContextForTemplateInstationPattern(const clang::Decl* D) {
     if(const auto* CTSD = dyn_cast<clang::ClassTemplateSpecializationDecl>(D->getDeclContext())) {
         return CTSD->getTemplateInstantiationPattern();

src/Compiler/Compilation.cpp

@@ -1,7 +1,12 @@
+
+#include "TidyImpl.h"
+
+#include "AST/Utility.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"
@@ -21,16 +26,17 @@ class ProxyASTConsumer final : public clang::MultiplexConsumer {
         src_mgr(instance.getSourceManager()), top_level_decls(top_level_decls), stop(stop) {}
 
     void collect_decl(clang::Decl* decl) {
-        auto location = decl->getLocation();
-        if(location.isInvalid()) {
+        if(!(ast::is_inside_main_file(decl->getLocation(), src_mgr))) {
             return;
         }
 
-        location = src_mgr.getExpansionLoc(location);
-        auto fid = src_mgr.getFileID(location);
-        if(fid == src_mgr.getPreambleFileID() || fid == src_mgr.getMainFileID()) {
-            top_level_decls->push_back(decl);
+        if(const clang::NamedDecl* named_decl = dyn_cast<clang::NamedDecl>(decl)) {
+            if(ast::is_implicit_template_instantiation(named_decl)) {
+                return;
+            }
         }
+
+        top_level_decls->push_back(decl);
     }
 
     auto HandleTopLevelDecl(clang::DeclGroupRef group) -> bool final {
@@ -158,10 +164,11 @@ CompilationResult run_clang(CompilationParams& params,
 
     auto diagnostics =
         params.diagnostics ? params.diagnostics : std::make_shared<std::vector<Diagnostic>>();
+    auto diagnostic_collector = Diagnostic::create(diagnostics);
     auto diagnostic_engine =
         clang::CompilerInstance::createDiagnostics(*params.vfs,
                                                    new clang::DiagnosticOptions(),
-                                                   Diagnostic::create(diagnostics));
+                                                   diagnostic_collector);
 
     auto invocation = create_invocation(params, diagnostic_engine);
     if(!invocation) {
@@ -193,15 +200,23 @@ 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 tidy_params;
+        checker = tidy::configure(*instance, tidy_params);
+        diagnostic_collector->checker = checker.get();
+    }
 
     /// `BeginSourceFile` may create new preprocessor, so all operations related to preprocessor
     /// should be done after `BeginSourceFile`.
@@ -239,6 +254,19 @@ 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) {
+        // 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(top_level_decls);
+        checker->finder.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.
 
@@ -247,10 +275,15 @@ CompilationResult run_clang(CompilationParams& params,
         resolver.emplace(instance->getSema());
     }
 
+    if(checker) {
+        /// Avoid dangling pointer.
+        diagnostic_collector->checker = nullptr;
+    }
+
     auto build_end = chrono::steady_clock::now().time_since_epoch();
 
     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.
+        std::shared_ptr<clang::Preprocessor> pp = instance->getPreprocessorPtr();
+        instance->setPreprocessor(nullptr);  // Detach so we don't send EOF again
         impl->action->EndSourceFile();
     }
 

src/Compiler/Diagnostic.cpp

@@ -1,4 +1,7 @@
 #include "Compiler/Diagnostic.h"
+#include "Support/Format.h"
+#include "TidyImpl.h"
+
 #include "clang/AST/Type.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
@@ -7,7 +10,6 @@
 #include "clang/Basic/AllDiagnostics.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Preprocessor.h"
-#include "Support/Format.h"
 
 namespace clice {
 
@@ -131,6 +133,10 @@ bool DiagnosticID::is_unused() const {
     return source == DiagnosticSource::Clang && unused_diags.contains(value);
 }
 
+bool is_note(clang::DiagnosticsEngine::Level level) {
+    return level == clang::DiagnosticsEngine::Note || level == clang::DiagnosticsEngine::Remark;
+}
+
 static DiagnosticLevel diagnostic_level(clang::DiagnosticsEngine::Level level) {
     switch(level) {
         case clang::DiagnosticsEngine::Ignored: return DiagnosticLevel::Ignored;
@@ -194,9 +200,9 @@ auto diagnostic_range(const clang::Diagnostic& diagnostic, const clang::LangOpti
     };
 }
 
-class DiagnosticCollector : public clang::DiagnosticConsumer {
+class DiagnosticCollectorImpl : 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 {
@@ -209,6 +215,12 @@ class DiagnosticCollector : public clang::DiagnosticConsumer {
 
         auto& diagnostic = diagnostics->emplace_back();
         diagnostic.id.value = raw_diagnostic.getID();
+
+        if(!is_note(level)) {
+            if(checker) {
+                level = checker->adjust_level(level, raw_diagnostic);
+            }
+        }
         diagnostic.id.level = diagnostic_level(level);
 
         /// TODO:
@@ -229,6 +241,10 @@ class DiagnosticCollector : public clang::DiagnosticConsumer {
             diagnostic.range = range;
         }
 
+        if(checker) {
+            checker->adjust_diag(diagnostic);
+        }
+
         /// TODO: handle FixIts
         /// raw_diagnostic.getFixItHints();
     }
@@ -241,9 +257,8 @@ class DiagnosticCollector : public clang::DiagnosticConsumer {
     clang::SourceManager* src_mgr;
 };
 
-clang::DiagnosticConsumer*
-    Diagnostic::create(std::shared_ptr<std::vector<Diagnostic>> diagnostics) {
-    return new DiagnosticCollector(diagnostics);
+DiagnosticCollector* Diagnostic::create(std::shared_ptr<std::vector<Diagnostic>> diagnostics) {
+    return new DiagnosticCollectorImpl(diagnostics);
 }
 
 }  // namespace clice