[Frontend] Avoid running plugins during code completion parse

[nik]

Summary

The parsing that is done for code completion is a special case that will
discard any generated diagnostics, so avoid running plugins for this
case in the first place to avoid performance penalties due to the
plugins.

A scenario for this is for example libclang with extra plugins like tidy.

[nik]

only clang-format fixes

[thakis]

Seems reasonable; can you add a test for this (maybe somewhere in clang/test/Frontend/plugin*)?

[john.brawn]

I know very little about how code completion works, but it's not immediately obvious to me that disabling plugin ast consumers when code completion is enabled is necessarily correct. In what kind of scenario would we both have a plugin loaded that wants to insert an ast consumer before/after the main one, and also be doing code completion?

[nik]

In D46050#1078112, @thakis wrote:

Seems reasonable; can you add a test for this (maybe somewhere in clang/test/Frontend/plugin*)?

Done.

In D46050#1078155, @john.brawn wrote:

I know very little about how code completion works, but it's not immediately obvious to me that disabling plugin ast consumers when code completion is enabled is necessarily correct. In what kind of scenario would we both have a plugin loaded that wants to insert an ast consumer before/after the main one, and also be doing code completion?

For example if you use libclang with the clang tidy plugin. I've put this into the commit message.

[nik]

Added a test and clarified scenario in commit message.

[nik]

test/Frontend/plugins.c

7 ↗

(On Diff #144084)

Should this test rather go into test/Index because of the c-index-test?

8 ↗

(On Diff #144084)

Note that I actually have problems with this REQUIRES line. I use -DCLANG_BUILD_EXAMPLES and -DDBUILD_SHARED_LIBS=ON and this test (and the one above too) is skipped/unsupported. What else do I need?

Note that If I remove this line, the test is run - apparently the requirements are fulfilled, but not properly detected. I guess this is set up issue on my local machine?

[thakis]

test/Frontend/plugins.c

7 ↗

(On Diff #144084)

Yes, I think so.

8 ↗

(On Diff #144084)

Are you on Windows? plugins is set here: http://llvm-cs.pcc.me.uk/tools/clang/test/lit.cfg.py#73
enable_shared here: http://llvm-cs.pcc.me.uk/tools/clang/test/lit.site.cfg.py.in#25
from here: http://llvm-cs.pcc.me.uk/cmake/modules/AddLLVM.cmake#1193

So my guess is you need to set LLVM_ENABLE_PLUGINS (and that you are on Windows), but I haven't tried it.

[nik]

test/Frontend/plugins.c

8 ↗

(On Diff #144084)

No, I'm not on Windows. Adding some debug-prints to the python scripts it looks like the "examples" thingy is actually not fulfilled here. Anyway, that's something for another change (will upload a separate change for this)

[nik]

Moved the test to "Index".

[nik]

OK, the issue with the test dependencies I've had is resolved by https://reviews.llvm.org/D46514

[nik]

Ping

[thakis]

test/Index/complete-and-plugins.c

4

nit: usually the RUN lines go above the C code, not below it.

[nik]

Addressed inline nit.

[nik]

If this is fine now, please submit as I don't have the permissions to do so.

lib/Frontend/FrontendAction.cpp

 std::unique_ptr<ASTConsumer>
 FrontendAction::CreateWrappedASTConsumer(CompilerInstance &CI,
                                          StringRef InFile) {
   std::unique_ptr<ASTConsumer> Consumer = CreateASTConsumer(CI, InFile);
   if (!Consumer)
     return nullptr;
 
   // If there are no registered plugins we don't need to wrap the consumer
-  if (FrontendPluginRegistry::begin() == FrontendPluginRegistry::end())
-    return Consumer;
-
-  // Collect the list of plugins that go before the main action (in Consumers)
-  // or after it (in AfterConsumers)
-  std::vector<std::unique_ptr<ASTConsumer>> Consumers;
+  if (FrontendPluginRegistry::begin() == FrontendPluginRegistry::end())
+    return Consumer;
+
+  // If this is a code completion run, avoid invoking the plugin consumers
+  if (CI.hasCodeCompletionConsumer())
+    return Consumer;
+
+  // Collect the list of plugins that go before the main action (in Consumers)
+  // or after it (in AfterConsumers)
+  std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   std::vector<std::unique_ptr<ASTConsumer>> AfterConsumers;
   for (FrontendPluginRegistry::iterator it = FrontendPluginRegistry::begin(),
                                         ie = FrontendPluginRegistry::end();
        it != ie; ++it) {
     std::unique_ptr<PluginASTAction> P = it->instantiate();
     PluginASTAction::ActionType ActionType = P->getActionType();
     if (ActionType == PluginASTAction::Cmdline) {
       // This is O(|plugins| * |add_plugins|), but since both numbers are

test/Index/complete-and-plugins.c

+// RUN: c-index-test -code-completion-at=%s:7:1 -load %llvmshlibdir/PrintFunctionNames%pluginext -add-plugin print-fns %s | FileCheck %s
+// REQUIRES: plugins, examples
+// CHECK: macro definition:{{.*}}
+// CHECK-NOT: top-level-decl: "x"
+
+void x();