sammccall added a comment.
Herald added a project: clang-tools-extra.
I'm getting a little nervous about the amount of stuff we're packing into
modules without in-tree examples.
I should split out some of the "standard" features into modules as that's
possible already.
My model for modules using this diagnostic stuff (apart from the build-system
stuff which sadly can't be meaningfully upstreamed) are IncludeFixer,
ClangTidy, and IWYU - worth thinking about how we'd build those on top of this
model. (Doesn't mean we need to add a hook to emit diagnostics, but it probably
means we should know where it would go)
================
Comment at: clang-tools-extra/clangd/FeatureModule.h:104
+ /// implementations don't need to be thread-safe.
+ struct ParseASTHooks {
+ virtual ~ParseASTHooks() = default;
----------------
naming: giving this a plural name and having a collection of them is a little
confusing (is `Hooks` the hooks from one module, or from all of them?). What
about ASTListener?
(Listener suggests passive but listeners that "participate" is a common enough
idiom I think)
================
Comment at: clang-tools-extra/clangd/FeatureModule.h:112
+ };
+ /// Can be called asynchronously before building an AST.
+ virtual std::unique_ptr<ParseASTHooks> astHooks() { return nullptr; }
----------------
This comment hints at what I *thought* was the idea: astHooks() is called each
time we parse a file, the returned object has methods called on it while the
file is being parsed, and is then destroyed.
But the code suggests we call once globally and it has effectively the same
lifetime as the module.
This seems much less useful, e.g. if we want to observe several diagnostics,
examine the preamble, and emit new diagnostics, then we have to plumb around
some notion of "AST identity" rather than just tying it to the identity of the
ParseASTHooks object itself.
(Lots of natural extensions here like providing ParseInputs to astHooks(), but
YAGNI for now)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98499/new/
https://reviews.llvm.org/D98499
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
