[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 20 04:02:16 PDT 2021
aaron.ballman added a comment.
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.
================
Comment at: clang/include/clang/AST/Decl.h:620
+ const DeclContext *Parent = getParent();
+ while (Parent->isTransparentContext())
+ Parent = Parent->getParent();
----------------
erichkeane wrote:
> This loop seems useful enough to be its own function in DeclContext? I think I remember seeing us do this for a different patch, right?
This pattern does appear in the code base a few places; I could add a new helper method.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108403/new/
https://reviews.llvm.org/D108403
More information about the cfe-commits
mailing list