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

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

Reply via email to