rjmccall added a comment.
Hmm. It sounds like we misbehave if we're working with a non-canonical ivar.
While it does seem preferable in general for lookups done after merging modules
to return the canonical ivar, I'm not sure we can rely on the AST only having
references to that ivar, since e.g. there might be references to non-canonical
ivars in static/inline functions in the modules that declare them. So while
this seems like a good change in the abstract — although maybe we should just
be doing the lookup in the canonical definition in the first place? — it also
might not be enough, and the record-layout code might just need to properly
handle non-canonical ivars.
================
Comment at: clang/lib/AST/DeclObjC.cpp:81-84
lookup_result R = lookup(Id);
for (lookup_iterator Ivar = R.begin(), IvarEnd = R.end();
Ivar != IvarEnd; ++Ivar) {
+ if (auto *ivar = dyn_cast<ObjCIvarDecl>((*Ivar)->getCanonicalDecl()))
----------------
vsapsai wrote:
> Don't really know what are the common patterns for name lookup and if this
> approach is acceptable. For C we call `LookupResult::resolveKind` to deal
> with multiple decls (and to avoid ambiguous lookup errors) but that's not
> available for `lookup_result` (aka `DeclContextLookupResult`).
I think this is fine given the specific constraints on this lookup and on
well-formed `@interface` declarations.
================
Comment at: clang/lib/Serialization/ASTReaderDecl.cpp:3354
+ if (auto *OID = dyn_cast<ObjCInterfaceDecl>(DC))
+ return OID->getDefinition();
+
----------------
This is fine if this function is only ever called when there's actually a known
member of the ID, which I think is true.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110280/new/
https://reviews.llvm.org/D110280
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
