sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/Module.h:97
+ /// Called by auxilary threads as diagnostics encountered to generate fixes.
+ /// So this can be called concurrently from multiple threads. If a module
----------------
This seems too specialized to me - maybe we want to observe diagnostics only,
or mutate some other property of them.
Consider just passing in the (mutable) clangd::Diag, and returning void? Maybe
call it `handleDiagnostic` or so
Maybe also the clang::Diagnostic if you really need it.
================
Comment at: clang-tools-extra/clangd/Module.h:98
+ /// Called by auxilary threads as diagnostics encountered to generate fixes.
+ /// So this can be called concurrently from multiple threads. If a module
+ /// provides a code action with a custom, it should also register itself as
----------------
random thoughts about how this gets called... (definitely not for now though!)
There are other things we might want to do while building ASTs.
e.g. *generate* diagnostics (maybe IWYU or clang-tidy can be modules)
At that point maybe we want something like
```
virtual unique_ptr<ParseASTHooks> Module::parseAST() { return nullptr; } //
threadsafe
class ParseASTHooks { // implementations need not be threadsafe
virtual void sawDiagnostic(clangd::Diag &D);
virtual void finish(ParsedAST&);
};
```
That way it's easier to reason about call sequences. (and it eliminates the
problem of calling an empty virtual method on each module for each diagnostic)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97555/new/
https://reviews.llvm.org/D97555
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
