Narrow inline namespace filtration for unqualified friend declarations

[troyj]

Summary

rG04ba1856 introduced a call to FilterLookupForScope that is expensive for very large translation units where it was observed to cause a 6% compile time degradation. As the comment states, the only effect is to "remove declarations found in inline namespaces for friend declarations with unqualified names." This change limits the call to that scenario. The test that was added by rG04ba1856 continues to pass, but the observed degradation is cut in half.

[troyj]

Pulled and regenerated diff. I'm not sure what the problem was and it still looks the same.

[bruno]

Hi Troy, thanks for working on this, nice compile time perf savings. Can you update the diff to contain more context? If anything, -U 999999 or similar would do the job.

@aaron.ballman wdyt?

[shafik]

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

2277–2278

Nitpick /*Scope=*/nullptr and /*ConsiderLinkage=*/true

[troyj]

Added more diff context as requested.

[aaron.ballman]

LGTM! Should we also add a release note or do we think this isn't enough of a compile-time performance improvement to warrant that?

[bruno]

LGTM! Should we also add a release note or do we think this isn't enough of a compile-time performance improvement to warrant that?

Good point @aaron.ballman, given we have only observed in a few corner case large TUs I'd say it's probably not that normally perceivable. Do you agree @troyj ?

[troyj]

I agree. I wouldn't want to mislead people into thinking that they'll see this improvement on everything.

[shafik]

It looks like this causes a regression, we can see in this bug report: https://github.com/llvm/llvm-project/issues/61851

[shafik]

A quick test indicated the !FunctionTemplate is the culprit for the noted regression.

[shafik]

PR for fix: https://reviews.llvm.org/D147762

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

     // tag type. Note that this does does not apply if we're declaring a
     // typedef (C++ [dcl.typedef]p4).
     if (Previous.isSingleTagDecl())
       Previous.clear();
 
     // Filter out previous declarations that don't match the scope. The only
     // effect this has is to remove declarations found in inline namespaces
     // for friend declarations with unqualified names.
-    SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr,
-                                 /*ConsiderLinkage*/ true,
-                                 QualifierLoc.hasQualifier());
+    if (isFriend && !QualifierLoc && !FunctionTemplate) {
+      SemaRef.FilterLookupForScope(Previous, DC, /*Scope=*/ nullptr,
+                                   /*ConsiderLinkage=*/ true,
+                                   QualifierLoc.hasQualifier());
+    }
   }
 
   // Per [temp.inst], default arguments in function declarations at local scope
   // are instantiated along with the enclosing declaration. For example:
   //
   //   template<typename T>
   //   void ft() {
   //     void f(int = []{ return T::value; }());