PR40329: [adl] Fix determination of associated classes when searching a

member enum and then its enclosing class.

There are situations where ADL will collect a class but not the complete
set of associated classes / namespaces of that class. When that
happened, and we later tried to collect those associated classes /
namespaces, we would previously short-circuit the lookup and not find
them. Eg, for:

  struct A : B { enum E; };

if we first looked for associated classes/namespaces of A::E, we'd find
only A. But if we then tried to also collect associated
classes/namespaces of A (which should include the base class B), we
would not add B because we had already visited A.

This also fixes a minor issue where we would fail to collect associated
classes from an overloaded class member access expression naming a
static member function.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@351382 91177308-0d34-0410-b5e6-96231b3b80d8

Author: zygoloid

lib/Sema/SemaLookup.cpp

@@ -2417,10 +2417,18 @@ namespace {
         InstantiationLoc(InstantiationLoc) {
     }
 
+    bool addClassTransitive(CXXRecordDecl *RD) {
+      Classes.insert(RD);
+      return ClassesTransitive.insert(RD);
+    }
+
     Sema &S;
     Sema::AssociatedNamespaceSet &Namespaces;
     Sema::AssociatedClassSet &Classes;
     SourceLocation InstantiationLoc;
+
+  private:
+    Sema::AssociatedClassSet ClassesTransitive;
   };
 } // end anonymous namespace
 
@@ -2521,15 +2529,6 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
   // Add the associated namespace for this class.
   CollectEnclosingNamespace(Result.Namespaces, Ctx);
 
-  // Add the class itself. If we've already seen this class, we don't
-  // need to visit base classes.
-  //
-  // FIXME: That's not correct, we may have added this class only because it
-  // was the enclosing class of another class, and in that case we won't have
-  // added its base classes yet.
-  if (!Result.Classes.insert(Class))
-    return;
-
   // -- If T is a template-id, its associated namespaces and classes are
   //    the namespace in which the template is defined; for member
   //    templates, the member template's class; the namespaces and classes
@@ -2552,6 +2551,11 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
       addAssociatedClassesAndNamespaces(Result, TemplateArgs[I]);
   }
 
+  // Add the class itself. If we've already transitively visited this class,
+  // we don't need to visit base classes.
+  if (!Result.addClassTransitive(Class))
+    return;
+
   // Only recurse into base classes for complete types.
   if (!Result.S.isCompleteType(Result.InstantiationLoc,
                                Result.S.Context.getRecordType(Class)))
@@ -2577,7 +2581,7 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
       if (!BaseType)
         continue;
       CXXRecordDecl *BaseDecl = cast<CXXRecordDecl>(BaseType->getDecl());
-      if (Result.Classes.insert(BaseDecl)) {
+      if (Result.addClassTransitive(BaseDecl)) {
         // Find the associated namespace for this base class.
         DeclContext *BaseCtx = BaseDecl->getDeclContext();
         CollectEnclosingNamespace(Result.Namespaces, BaseCtx);
@@ -2793,15 +2797,9 @@ void Sema::FindAssociatedClassesAndNamespaces(
     // in which the function or function template is defined and the
     // classes and namespaces associated with its (non-dependent)
     // parameter types and return type.
-    Arg = Arg->IgnoreParens();
-    if (UnaryOperator *unaryOp = dyn_cast<UnaryOperator>(Arg))
-      if (unaryOp->getOpcode() == UO_AddrOf)
-        Arg = unaryOp->getSubExpr();
-
-    UnresolvedLookupExpr *ULE = dyn_cast<UnresolvedLookupExpr>(Arg);
-    if (!ULE) continue;
+    OverloadExpr *OE = OverloadExpr::find(Arg).Expression;
 
-    for (const auto *D : ULE->decls()) {
+    for (const NamedDecl *D : OE->decls()) {
       // Look through any using declarations to find the underlying function.
       const FunctionDecl *FDecl = D->getUnderlyingDecl()->getAsFunction();
 

test/CXX/drs/dr0xx.cpp

@@ -413,6 +413,36 @@ namespace dr33 { // dr33: yes
   void g(X::S);
   template<typename Z> Z g(Y::T);
   void h() { f(&g); } // expected-error {{ambiguous}}
+
+  template<typename T> void t(X::S);
+  template<typename T, typename U = void> void u(X::S); // expected-error 0-1{{default template argument}}
+  void templ() { f(t<int>); f(u<int>); }
+
+  // Even though v<int> cannot select the first overload, ADL considers it
+  // and adds namespace Z to the set of associated namespaces, and then picks
+  // Z::f even though that function has nothing to do with any associated type.
+  namespace Z { struct Q; void f(void(*)()); }
+  template<int> Z::Q v();
+  template<typename> void v();
+  void unrelated_templ() { f(v<int>); }
+
+  namespace dependent {
+    struct X {};
+    template<class T> struct Y {
+      friend int operator+(X, void(*)(Y)) {}
+    };
+
+    template<typename T> void f(Y<T>);
+    int use = X() + f<int>; // expected-error {{invalid operands}}
+  }
+
+  namespace member {
+    struct Q {};
+    struct Y { friend int operator+(Q, Y (*)()); };
+    struct X { template<typename> static Y f(); };
+    int m = Q() + X().f<int>; // ok
+    int n = Q() + (&(X().f<int>)); // ok
+  }
 }
 
 // dr34: na

test/SemaCXX/adl.cpp

@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -verify %s
+
+namespace PR40329 {
+  struct A {
+    A(int);
+    friend int operator->*(A, A);
+  };
+  struct B : A {
+    B();
+    enum E { e };
+  };
+  // Associated classes for B are {B, A}
+  // Associated classes for B::E are {B} (non-transitive in this case)
+  //
+  // If we search B::E first, we must not mark B "visited" and shortcircuit
+  // visiting it later, or we won't find the associated class A.
+  int k0 = B::e ->* B::e; // expected-error {{non-pointer-to-member type}}
+  int k1 = B::e ->* B();
+  int k2 = B() ->* B::e;
+}