[PATCH] D97555: [clangd] Add diagnostic augmentation hook to Modules

[Sam McCall via Phabricator via cfe-commits]

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