[PATCH] D83224: [clangd] Move clang-tidy check modifications into ClangdServer

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, aaron.ballman, usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

This enables sharing the logic between standalone clangd and embedders
of it. The new approach should be same performance-wise, as it is only called
once per addDocument call.

This patch also introduces a blacklisting code path for disabling crashy or
high-noise tests, until we figure out a way to make them work with clangd-setup.

The biggest difference is the way we make use of preambles, hence those checks
can't see directives coming from the preamble section of the file. The second
thing is the fact that code might-not be compiling while clangd is trying to
build an AST, hence some checks might choke on those incomplete ASTs.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83224

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===================================================================
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -729,23 +729,6 @@
         // FIXME: use the FS provided to the function.
         Opts = ClangTidyOptProvider->getOptions(File);
       }
-      if (!Opts.Checks) {
-        // If the user hasn't configured clang-tidy checks at all, including
-        // via .clang-tidy, give them a nice set of checks.
-        // (This should be what the "default" options does, but it isn't...)
-        //
-        // These default checks are chosen for:
-        //  - low false-positive rate
-        //  - providing a lot of value
-        //  - being reasonably efficient
-        Opts.Checks = llvm::join_items(
-            ",", "readability-misleading-indentation",
-            "readability-deleted-default", "bugprone-integer-division",
-            "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma",
-            "bugprone-unused-raii", "bugprone-unused-return-value",
-            "misc-unused-using-decls", "misc-unused-alias-decls",
-            "misc-definitions-in-headers");
-      }
       return Opts;
     };
   }
Index: clang-tools-extra/clangd/ClangdServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -107,6 +107,17 @@
   ClangdServer::Callbacks *ServerCallbacks;
   bool TheiaSemanticHighlighting;
 };
+
+// Set of clang-tidy checks that are not suitable to be run through clangd,
+// either due to crashes or false positives.
+std::string getClangTidyBlacklist() {
+  std::string FalsePositives =
+      llvm::join_items(", ",
+                       // Check relies on seeing ifndef/define/endif directives,
+                       // clangd doesn't replay those when using a preamble.
+                       "-llvm-header-guard");
+  return llvm::join_items(", ", FalsePositives);
+}
 } // namespace
 
 ClangdServer::Options ClangdServer::optsForTest() {
@@ -186,6 +197,28 @@
   if (GetClangTidyOptions)
     Opts.ClangTidyOpts =
         GetClangTidyOptions(*TFS.view(/*CWD=*/llvm::None), File);
+  if (!Opts.ClangTidyOpts.Checks) {
+    // If the user hasn't configured clang-tidy checks at all, including
+    // via .clang-tidy, give them a nice set of checks.
+    // (This should be what the "default" options does, but it isn't...)
+    //
+    // These default checks are chosen for:
+    //  - low false-positive rate
+    //  - providing a lot of value
+    //  - being reasonably efficient
+    Opts.ClangTidyOpts.Checks = llvm::join_items(
+        ",", "readability-misleading-indentation",
+        "readability-deleted-default", "bugprone-integer-division",
+        "bugprone-sizeof-expression", "bugprone-suspicious-missing-comma",
+        "bugprone-unused-raii", "bugprone-unused-return-value",
+        "misc-unused-using-decls", "misc-unused-alias-decls",
+        "misc-definitions-in-headers");
+  } else {
+    // If user has enabled some checks, make sure clangd incompatible ones are
+    // disabled.
+    Opts.ClangTidyOpts.Checks = llvm::join_items(
+        ", ", *Opts.ClangTidyOpts.Checks, getClangTidyBlacklist());
+  }
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
 
   // Compile command is set asynchronously during update, as it can be slow.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D83224.275699.patch
Type: text/x-patch
Size: 3465 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200706/a814262f/attachment-0001.bin>