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 fully internalized the issue you're solving > > here, but this is a very performance-sensitive path in the compiler; > > there's a reason this code is written to very carefully only do extra work > > when we've detected in the loop below that we're in a hidden-declarations > > situation. Is there any way we can restore that basic structure? > Test4 in the added tests is an example of why we can't wait until after the > main loop. The `using A::X` in namespace D is eliminated by the UniqueResult > check, so the check for a declaration being hidden can only see the using > declarations in namespace C. We also can't do it in the loop itself, as then > we can't handle Test5: at the time we process the `using A::X` in namespace D > it looks like it may cause ambiguity, but it's later hidden by the `using > B::X` in the same namespace which we haven't yet processed. > > I have adjusted it though so the nested loop and erasing of decls only > happens when we both have things that hide and things that can be hidden. > Doing some quick testing of compiling SemaOpenMP.cpp (the largest file in > clang), LookupResult::resolveKind is called 75318 times, of which 75283 calls > have HideTags=true, of which 56 meet this condition, i.e. 0.07%. Okay, I can see why you need to not mix tag-hiding with the removal of duplicates. However, I think you can maintain the current structure by delaying the actual removal of declarations until after the main loop; have the loop build up a set of indices to remove instead. (Also, you can keep this set as a bitset instead of a `std::set<unsigned>`.) It's a shame that the hiding algorithm has to check every other declaration in the lookup in case they're from different scopes. I guess to avoid that we'd have to do the filtering immediately when we collect the declarations from a particular DC. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
