[PATCH] D90975: [clangd] Don't run clang-tidy AST traversal if there are no checks.

2020-11-08 Thread Sam McCall via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG053110b22aa9: [clangd] Dont run clang-tidy AST 
traversal if there are no checks. (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90975/new/

https://reviews.llvm.org/D90975

Files:
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -303,9 +303,18 @@
 CTContext->setASTContext(>getASTContext());
 CTContext->setCurrentFile(Filename);
 CTChecks = CTFactories.createChecks(CTContext.getPointer());
-ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
-   const clang::Diagnostic ) {
-  if (CTContext) {
+llvm::erase_if(CTChecks, [&](const auto ) {
+  return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
+});
+Preprocessor *PP = >getPreprocessor();
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
+  Check->registerMatchers();
+}
+
+if (!CTChecks.empty()) {
+  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level 
DiagLevel,
+ const clang::Diagnostic ) {
 std::string CheckName = CTContext->getCheckName(Info.getID());
 bool IsClangTidyDiag = !CheckName.empty();
 if (IsClangTidyDiag) {
@@ -330,17 +339,8 @@
 return DiagnosticsEngine::Error;
   }
 }
-  }
-  return DiagLevel;
-});
-Preprocessor *PP = >getPreprocessor();
-for (const auto  : CTChecks) {
-  if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
-continue;
-  // FIXME: the PP callbacks skip the entire preamble.
-  // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
-  Check->registerMatchers();
+return DiagLevel;
+  });
 }
   }
 
@@ -415,7 +415,7 @@
   std::vector ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
-  {
+  if (!CTChecks.empty()) {
 // Run the AST-dependent part of the clang-tidy checks.
 // (The preprocessor part ran already, via PPCallbacks).
 trace::Span Tracer("ClangTidyMatch");


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -303,9 +303,18 @@
 CTContext->setASTContext(>getASTContext());
 CTContext->setCurrentFile(Filename);
 CTChecks = CTFactories.createChecks(CTContext.getPointer());
-ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
-   const clang::Diagnostic ) {
-  if (CTContext) {
+llvm::erase_if(CTChecks, [&](const auto ) {
+  return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
+});
+Preprocessor *PP = >getPreprocessor();
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
+  Check->registerMatchers();
+}
+
+if (!CTChecks.empty()) {
+  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic ) {
 std::string CheckName = CTContext->getCheckName(Info.getID());
 bool IsClangTidyDiag = !CheckName.empty();
 if (IsClangTidyDiag) {
@@ -330,17 +339,8 @@
 return DiagnosticsEngine::Error;
   }
 }
-  }
-  return DiagLevel;
-});
-Preprocessor *PP = >getPreprocessor();
-for (const auto  : CTChecks) {
-  if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
-continue;
-  // FIXME: the PP callbacks skip the entire preamble.
-  // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
-  Check->registerMatchers();
+return DiagLevel;
+  });
 }
   }
 
@@ -415,7 +415,7 @@
   std::vector ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
-  {
+  if (!CTChecks.empty()) {
 // Run the AST-dependent part of the clang-tidy checks.
 // (The preprocessor part ran already, via PPCallbacks).
 trace::Span Tracer("ClangTidyMatch");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90975: [clangd] Don't run clang-tidy AST traversal if there are no checks.

2020-11-07 Thread Nathan James via Phabricator via cfe-commits 
njames93 accepted this revision.
njames93 added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90975/new/

https://reviews.llvm.org/D90975

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D90975: [clangd] Don't run clang-tidy AST traversal if there are no checks.

2020-11-06 Thread Sam McCall via Phabricator via cfe-commits 
sammccall created this revision.
sammccall added a reviewer: njames93.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman.
Herald added a project: clang.
sammccall requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

While here, clean up ParsedAST::build a bit:

- remove FIXMEs that were fixed long ago by ReplayPreamble
- remove redundant if, ClangTidyContext is not actually optional


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90975

Files:
  clang-tools-extra/clangd/ParsedAST.cpp


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -303,9 +303,18 @@
 CTContext->setASTContext(>getASTContext());
 CTContext->setCurrentFile(Filename);
 CTChecks = CTFactories.createChecks(CTContext.getPointer());
-ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
-   const clang::Diagnostic ) {
-  if (CTContext) {
+llvm::erase_if(CTChecks, [&](const auto ) {
+  return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
+});
+Preprocessor *PP = >getPreprocessor();
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
+  Check->registerMatchers();
+}
+
+if (!CTChecks.empty()) {
+  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level 
DiagLevel,
+ const clang::Diagnostic ) {
 std::string CheckName = CTContext->getCheckName(Info.getID());
 bool IsClangTidyDiag = !CheckName.empty();
 if (IsClangTidyDiag) {
@@ -330,17 +339,8 @@
 return DiagnosticsEngine::Error;
   }
 }
-  }
-  return DiagLevel;
-});
-Preprocessor *PP = >getPreprocessor();
-for (const auto  : CTChecks) {
-  if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
-continue;
-  // FIXME: the PP callbacks skip the entire preamble.
-  // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
-  Check->registerMatchers();
+return DiagLevel;
+  });
 }
   }
 
@@ -415,7 +415,7 @@
   std::vector ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
-  {
+  if (!CTChecks.empty()) {
 // Run the AST-dependent part of the clang-tidy checks.
 // (The preprocessor part ran already, via PPCallbacks).
 trace::Span Tracer("ClangTidyMatch");


Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -303,9 +303,18 @@
 CTContext->setASTContext(>getASTContext());
 CTContext->setCurrentFile(Filename);
 CTChecks = CTFactories.createChecks(CTContext.getPointer());
-ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
-   const clang::Diagnostic ) {
-  if (CTContext) {
+llvm::erase_if(CTChecks, [&](const auto ) {
+  return !Check->isLanguageVersionSupported(CTContext->getLangOpts());
+});
+Preprocessor *PP = >getPreprocessor();
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
+  Check->registerMatchers();
+}
+
+if (!CTChecks.empty()) {
+  ASTDiags.setLevelAdjuster([](DiagnosticsEngine::Level DiagLevel,
+ const clang::Diagnostic ) {
 std::string CheckName = CTContext->getCheckName(Info.getID());
 bool IsClangTidyDiag = !CheckName.empty();
 if (IsClangTidyDiag) {
@@ -330,17 +339,8 @@
 return DiagnosticsEngine::Error;
   }
 }
-  }
-  return DiagLevel;
-});
-Preprocessor *PP = >getPreprocessor();
-for (const auto  : CTChecks) {
-  if (!Check->isLanguageVersionSupported(CTContext->getLangOpts()))
-continue;
-  // FIXME: the PP callbacks skip the entire preamble.
-  // Checks that want to see #includes in the main file do not see them.
-  Check->registerPPCallbacks(Clang->getSourceManager(), PP, PP);
-  Check->registerMatchers();
+return DiagLevel;
+  });
 }
   }
 
@@ -415,7 +415,7 @@
   std::vector ParsedDecls = Action->takeTopLevelDecls();
   // AST traversals should exclude the preamble, to avoid performance cliffs.
   Clang->getASTContext().setTraversalScope(ParsedDecls);
-  {
+  if (!CTChecks.empty()) {
 // Run the AST-dependent part of the clang-tidy checks.
 // (The preprocessor part ran already, via PPCallbacks).
 trace::Span Tracer("ClangTidyMatch");