erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land.
In D108403#2957084 <https://reviews.llvm.org/D108403#2957084>, @aaron.ballman wrote: > In D108403#2955651 <https://reviews.llvm.org/D108403#2955651>, @erichkeane > wrote: > >> This whole function seems a little suspect, but I don't have a good example >> of a place it would break. Is there no cases where a lookup could result in >> the same COUNT but different declaration set? I guess it is more the >> question of whether a transparent context can 'lose' a name lookup (perhaps >> a case of conflicting names?), then have it added by the local namespace. > > I don't believe transparent contexts can lose a name lookup -- they're > transparent, so the declarations aren't owned by that context, they're owned > by the first non-transparent context they run into (as I understand it, > anyway). However, the important bit on this patch is: `lookup()` asserts that > you're not trying to call it on a transparent context, so that tells me that > we should walk until we find a non-transparent context rather than the > current approach of assuming the parent is not transparent. Right... you're probably right here, the mechanism with 'lookup' just feels 1/2 too clever. Anyway, I think I believe that is a battle for another day. See the clang-format warning above, otherwise LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108403/new/ https://reviews.llvm.org/D108403 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
