[PATCH] D94987: DR39: Perform ambiguous subobject checks for class member access as part of object argument conversion, not as part of name lookup.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D94987#2521774 <https://reviews.llvm.org/D94987#2521774>, @rjmccall wrote:

>> You can. The rule is that you resolve using-declarations to the declarations they name, and you resolve typedef declarations to the types they name, and the resulting set of declarations and types must be the same in every subobject in which there are (non-shadowed) results -- if not, the lookup is ill-formed. (Put another way, the set of entities found in each subobject must be the same, but the set of declarations found in each subobject can differ.)
>
> Okay, makes sense.  So the DeclAccessPairs in the lookup result are basically just the first+best pair we found for any particular entity?

Yes, each pair comprises the declaration from the first subobject, with the best unprivileged access for the corresponding declarations in all subobjects.



================
Comment at: clang/include/clang/AST/CXXInheritance.h:77
 
-  CXXBasePath() = default;
+  /// Additional data stashed on the base path by its consumers.
+  union {
----------------
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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D94987/new/

https://reviews.llvm.org/D94987