nridge accepted this revision.
nridge added a comment.
This revision is now accepted and ready to land.
Not sure if I'm authorized to approve changes to libTooling, but this LGTM!
================
Comment at: clang/lib/Tooling/CompilationDatabase.cpp:73
CompilationDatabasePluginRegistry::entries()) {
+ Plugins.emplace_back(Database.getName(), Database.instantiate());
+ }
----------------
njames93 wrote:
> nridge wrote:
> > A side effect of this change is that every plugin will get instantiated,
> > not just the ones for which we attempt a load.
> >
> > Perhaps we should store a pointer to the entry in the vector here, and
> > instantiate() in the second loop below?
> The original setup was to instantiate every plugin until we find one that is
> able to load from the directory.
> As for your proposed solution, that wouldn't work as we can't get the
> priority without instantiating the plugin.
> The current registry system also doesn't lend itself nicely.
> I'm not too familiar, but it could be possible to specialise
> `SimpleRegistryEntry` for `CompilationDatabasePlugin`.
> that wouldn't work as we can't get the priority without instantiating the
> plugin
Right, of course, I overlooked that.
Anyways, it looks like instantiating the plugin just calls its constructor and
the constructors of both existing plugins are no-ops, so it shouldn't be a
concern.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91351/new/
https://reviews.llvm.org/D91351
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
