rjmccall added inline comments.
================
Comment at: clang/include/clang/AST/CXXInheritance.h:77
- CXXBasePath() = default;
+ /// Additional data stashed on the base path by its consumers.
+ union {
----------------
rsmith wrote:
> rjmccall wrote:
> > rsmith wrote:
> > > rjmccall wrote:
> > > > Is there a way to know which case we're in, or do different consumers
> > > > do different things?
> > > We could use a discriminated union here. Ultimately, I think what we
> > > really want is to provide some opaque storage for the consumer to use,
> > > rather than to have some hard-coded list here. I haven't thought of a
> > > good way to expose that; I don't think it's worth templating the entire
> > > base path finding mechanism to add such storage, and a single pointer
> > > isn't large enough for a `DeclContext::lookup_result`.
> > >
> > > Vassil's https://reviews.llvm.org/D91524 will make the `lookup_result`
> > > case fit into a single pointer; once we adopt that, perhaps we can switch
> > > to holding an opaque `void*` here?
> > Yeah, that might be cleaner, assuming we really need this. What are the
> > clients that need to store something specifically in the `CXXBasePath`
> > object and can't just keep it separate?
> The main users of this are in name lookup proper (where we store the lookup
> results on the path to avoid redoing the class scope hash table lookup) and
> now in access checking (where we store the most-accessible declaration). I
> think there's a couple of other name-lookup-like clients that also store a
> lookup result here.
>
> In all cases the side storage is just a convenience. But it's probably an
> important convenience; it's awkward for a client to maintain a mapping on the
> side, because the key for that map would end up being the entire base path.
>
> We could just number the paths as we find them, and let the consumers build a
> `SmallVector` on the side rather than storing data in the `CXXBasePath`s.
> We'd need to be careful about the post-processing pass in `lookupInBases`
> that removes virtual base paths that are shadowed by non-virtual inheritance
> from the vbase, but that seems feasible. Worthwhile?
I see what you're saying. We have places that compute all the base paths in an
array, and it's convenient to be able to stash things into the elements of that
array instead of having a second data structure that's parallel to it. I guess
there are three options:
- Tell clients to just make a parallel structure.
- Have this sort of intrusive storage and just accept that clients that want to
use it will have some awkward casting to do.
- Get rid of the initial array dependence by making it a callback API instead
of an array-building API, and then clients that want to build an array can
build an array with the type they actually want.
I guess the third might be awkward?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D94987/new/
https://reviews.llvm.org/D94987
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
