[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added a comment. I've committed an updated version of this that checks for invalid decls when deciding if a decl has been hidden. This stops the assertion failure happening. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ http

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added a comment. I've committed a test for the behaviour when we have invalid decls in https://reviews.llvm.org/rG6244e3840694. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

kadircet added a comment. In D154503#4576481 , @aaron.ballman wrote: > In D154503#4576475 , @kadircet > wrote: > >> In D154503#4576442 , >> @aaron.ballman wrote: >> >>>

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

aaron.ballman added a comment. In D154503#4576475 , @kadircet wrote: > In D154503#4576442 , @aaron.ballman > wrote: > >> I'm not opposed to a revert if this is causing problems for your downstream, >> but be sur

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

kadircet added a comment. In D154503#4576442 , @aaron.ballman wrote: > I'm not opposed to a revert if this is causing problems for your downstream, > but be sure to also remove the changes from the 17.x branch if you go this > route rather than fixing

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

aaron.ballman added a comment. In D154503#4576393 , @kadircet wrote: > Hi! > > After this patch clang seems to be crashing on: > > class a { > static c a; > void b(){a}; > }; > > with stack trace and assertion error: > > $ ~/repos/llvm/build

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

kadircet added a comment. Hi! After this patch clang seems to be crashing on: class a { static c a; void b(){a}; }; with stack trace and assertion error: $ ~/repos/llvm/build/bin/clang -xc++ repro.cc repro.cc:2:10: error: unknown type name 'c' 2 | static c a; |

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added a comment. The first version of this that I committed caused a failure in clang/test/Modules/stress1.cpp so I reverted it. I've now committed a new version that handles the removal of existing decl when isPreferredLookupResult is true in a slightly different way, which should

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGdfca88341794: [Sema] Fix handling of functions that hide classes (authored by john.brawn). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thank you, this looks great, and I really appreciate that you found a way to make it work with just the single loop (in the typical case). CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn updated this revision to Diff 542893. john.brawn edited the summary of this revision. john.brawn added a comment. Restructured to check for hidden tags in the main loop. Also add a bunch of extra tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://review

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:507 + // C++ [basic.scope.hiding]p2: + // A class name or enumeration name can be hidden by the name of john.brawn wrote: > shafik wrote: > > This section does not exist anymore, it was

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

rjmccall added a comment. I'm suggesting: llvm::BitVector declsToRemove; while (I < N) { if (shouldHideTag()) declsToRemove.add(I); if (notUnique()) declsToRemove.add(I); } Decls.removeAll(declsToRemove) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ ht

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + rjmccall wrote: > dexonsmith wrote: > > john.brawn wrote: > > > rjmccall wrote: > > > > john.brawn wrote: > > > > > rjmccall wrote: > > > > > > This is g

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + dexonsmith wrote: > john.brawn wrote: > > rjmccall wrote: > > > john.brawn wrote: > > > > rjmccall wrote: > > > > > This is going to fire on every single o

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn updated this revision to Diff 539142. john.brawn added a comment. Use BitVector, change how Decls are deleted, adjust test to avoid potentially-conflicting using-declarations. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 Files:

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

dexonsmith added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + john.brawn wrote: > rjmccall wrote: > > john.brawn wrote: > > > rjmccall wrote: > > > > This is going to fire on every single ordinary lookup that finds

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + rjmccall wrote: > john.brawn wrote: > > rjmccall wrote: > > > This is going to fire on every single ordinary lookup that finds multiple > > > declaratio

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:507 + // C++ [basic.scope.hiding]p2: + // A class name or enumeration name can be hidden by the name of shafik wrote: > This section does not exist anymore, it was replaced in > [p

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:507 + // C++ [basic.scope.hiding]p2: + // A class name or enumeration name can be hidden by the name of This section does not exist anymore, it was replaced in [p1787](https://wg21.lin

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added inline comments. Comment at: clang/test/SemaCXX/using-hiding.cpp:20 + +// Using declaration causes A::X to be hidden, so X is not ambiguous. +namespace Test2 { I think [namespace.udecl p10](https://eel.is/c++draft/namespace.udecl#10) disagrees, spec

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added reviewers: aaron.ballman, clang-language-wg. shafik added a comment. For more visibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + john.brawn wrote: > rjmccall wrote: > > This is going to fire on every single ordinary lookup that finds multiple > > declarations, right? I haven't full

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn updated this revision to Diff 537731. john.brawn added a comment. Same patch as previous, but with full context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 Files: clang/lib/Sema/SemaLookup.cpp clang/test/SemaCXX/using-hidin

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + rjmccall wrote: > This is going to fire on every single ordinary lookup that finds multiple > declarations, right? I haven't fully internalized the iss

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn updated this revision to Diff 537721. john.brawn added a comment. Avoid doing work when we don't have both decls that can hide and decls that can be hidden. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 Files: clang/lib/Sema/Se

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

rjmccall added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:542 +N = Decls.size(); + } + This is going to fire on every single ordinary lookup that finds multiple declarations, right? I haven't fully internalized the issue you're solving her

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

john.brawn created this revision. john.brawn added reviewers: dexonsmith, rsmith, rjmccall. Herald added a project: All. john.brawn requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When a function is declared in the same scope as a class wit