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

[John Brawn via Phabricator via cfe-commits]

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 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%.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154503/new/

https://reviews.llvm.org/D154503