[PATCH] D92155: Load plugins when creating a CompilerInvocation.

Fri Nov 27 01:25:47 PST 2020

psionic12 added a comment.

> - code completion: this shouldn't run plugins I believe: latency is critical and we don't emit any diagnostics

Totally agree.

> - main file AST build: Important to run plugins here but it's critical they don't traverse the whole preamble (can degrade latency by orders of magnitude).

It's rare that a plugin traverse preamble, most cases it deals with AST, but if it does (I guess using a `PragmaHandler`?), I just didn't come up with an idea how to prevent this...

> - preamble build: this is tricky, we *rarely* emit diagnostics from this step, and it's very performance-sensitive. It's a separate action from the main file AST build so plugins won't be able to see the results of analysis. I think we shouldn't run plugins here.

Save as above, it rare in writing plugins, either, but don't know how we can prevent it.

> - background indexing: again, no reason to run plugins here

Agree, but I got a question, what if a error diagnostic exist in the code, will the index procedure do?

> - we need to ensure the traversal scope is already set by the time plugin hooks that may perform traversals (e.g. ASTConsumer::handleTranslationUnit) are called

Do you mean for example a plugin code called `handleTranslationUnit()` not in callbacks, but in setup stage? Such as calling handleTranslationUnit() in constructors while the instance is a static variable?
This looks rare to me either. Plugins are driven by callbacks, if the context is not ready, callbacks won't get called. But if some programmer really access some resources that are not prepared at that time, it seems that we are back to the discussion that if a plugin crashes or do bad things, nothing we can do about that for now...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92155