[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, rsmith, rjmccall, jyknight. aaron.ballman requested review of this revision. Herald added a project: clang. When calculating the name to display for inline namespaces, we have custom logic to try to hide redundant inl

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-19 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. 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

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In 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 dif

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 367765. aaron.ballman added a comment. Updating based on review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108403/new/ https://reviews.llvm.org/D108403 Files: clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. In D108403#2957084 , @aaron.ballman wrote: > In D108403#2955651 , @erichkeane > wrote: > >> This wh

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman closed this revision. aaron.ballman marked an inline comment as done. aaron.ballman added a comment. Thanks for the reviews! I've committed (with the formatting fix) in 48f73ee666a264d23716ff6bb671cad836b65ccf .

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/AST/DeclBase.cpp:1222 + DeclContext *DC = this; + while (DC && DC->isTransparentContext()) +DC = DC->getParent(); Can getParent() be null? If it cam, then this function can return null which you don't c

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments. Comment at: clang/lib/AST/DeclBase.cpp:1222 + DeclContext *DC = this; + while (DC && DC->isTransparentContext()) +DC = DC->getParent(); cor3ntin wrote: > Can getParent() be null? If it cam, then this function can return nul

[PATCH] D108403: Fix assertion when generating diagnostic for inline namespaces

2021-08-20 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments. Comment at: clang/lib/AST/DeclBase.cpp:1222 + DeclContext *DC = this; + while (DC && DC->isTransparentContext()) +DC = DC->getParent(); erichkeane wrote: > cor3ntin wrote: > > Can getParent() be null? If it cam, then thi