Handle redeclarations found by ADL deterministically and reasonably.

This solution relies on an O(n) scan of redeclarations, which means it might
scale poorly in crazy cases with tons of redeclarations brought in by a ton
of distinct associated namespaces.  I believe that avoiding this
is not worth the common-case cost.

llvm-svn: 94530

clang/lib/Sema/Lookup.h

@@ -587,6 +587,44 @@ private:
     virtual void FoundDecl(NamedDecl *ND, NamedDecl *Hiding, 
                            bool InBaseClass) = 0;
   };
+
+/// \brief A class for storing results from argument-dependent lookup.
+class ADLResult {
+private:
+  /// A map from canonical decls to the 'most recent' decl.
+  llvm::DenseMap<NamedDecl*, NamedDecl*> Decls;
+
+public:
+  /// Adds a new ADL candidate to this map.
+  void insert(NamedDecl *D);
+
+  /// Removes any data associated with a given decl.
+  void erase(NamedDecl *D) {
+    Decls.erase(cast<NamedDecl>(D->getCanonicalDecl()));
+  }
+
+  class iterator {
+    typedef llvm::DenseMap<NamedDecl*,NamedDecl*>::iterator inner_iterator;
+    inner_iterator iter;
+
+    friend class ADLResult;
+    iterator(const inner_iterator &iter) : iter(iter) {}
+  public:
+    iterator() {}
+
+    iterator &operator++() { ++iter; return *this; }
+    iterator operator++(int) { return iterator(iter++); }
+
+    NamedDecl *operator*() const { return iter->second; }
+
+    bool operator==(const iterator &other) const { return iter == other.iter; }
+    bool operator!=(const iterator &other) const { return iter != other.iter; }
+  };
+
+  iterator begin() { return iterator(Decls.begin()); }
+  iterator end() { return iterator(Decls.end()); }
+};
+
 }
 
 #endif

clang/lib/Sema/Sema.h

@@ -103,6 +103,7 @@ namespace clang {
   class InitializationSequence;
   class VisibleDeclConsumer;
   class TargetAttributesSema;
+  class ADLResult;
 
 /// BlockSemaInfo - When a block is being parsed, this contains information
 /// about the block.  It is pointed to from Sema::CurBlock.
@@ -950,8 +951,6 @@ public:
   // Members have to be NamespaceDecl* or TranslationUnitDecl*.
   // TODO: make this is a typesafe union.
   typedef llvm::SmallPtrSet<DeclContext   *, 16> AssociatedNamespaceSet;
-  // Members have to be a function or function template.
-  typedef llvm::SmallPtrSet<NamedDecl*, 16> ADLFunctionSet;
   typedef llvm::SmallPtrSet<CXXRecordDecl *, 16> AssociatedClassSet;
 
   void AddOverloadCandidate(NamedDecl *Function,
@@ -1240,7 +1239,7 @@ public:
   
   void ArgumentDependentLookup(DeclarationName Name, bool Operator,
                                Expr **Args, unsigned NumArgs,
-                               ADLFunctionSet &Functions);
+                               ADLResult &Functions);
 
   void LookupVisibleDecls(Scope *S, LookupNameKind Kind,
                           VisibleDeclConsumer &Consumer);

clang/lib/Sema/SemaLookup.cpp

@@ -1731,9 +1731,46 @@ void Sema::LookupOverloadedOperatorName(OverloadedOperatorKind Op, Scope *S,
   }
 }
 
+void ADLResult::insert(NamedDecl *New) {
+  NamedDecl *&Old = Decls[cast<NamedDecl>(New->getCanonicalDecl())];
+
+  // If we haven't yet seen a decl for this key, or the last decl
+  // was exactly this one, we're done.
+  if (Old == 0 || Old == New) {
+    Old = New;
+    return;
+  }
+
+  // Otherwise, decide which is a more recent redeclaration.
+  FunctionDecl *OldFD, *NewFD;
+  if (isa<FunctionTemplateDecl>(New)) {
+    OldFD = cast<FunctionTemplateDecl>(Old)->getTemplatedDecl();
+    NewFD = cast<FunctionTemplateDecl>(New)->getTemplatedDecl();
+  } else {
+    OldFD = cast<FunctionDecl>(Old);
+    NewFD = cast<FunctionDecl>(New);
+  }
+
+  FunctionDecl *Cursor = NewFD;
+  while (true) {
+    Cursor = Cursor->getPreviousDeclaration();
+
+    // If we got to the end without finding OldFD, OldFD is the newer
+    // declaration;  leave things as they are.
+    if (!Cursor) return;
+
+    // If we do find OldFD, then NewFD is newer.
+    if (Cursor == OldFD) break;
+
+    // Otherwise, keep looking.
+  }
+
+  Old = New;
+}
+
 void Sema::ArgumentDependentLookup(DeclarationName Name, bool Operator,
                                    Expr **Args, unsigned NumArgs,
-                                   ADLFunctionSet &Functions) {
+                                   ADLResult &Result) {
   // Find all of the associated namespaces and classes based on the
   // arguments we have.
   AssociatedNamespaceSet AssociatedNamespaces;
@@ -1788,17 +1825,15 @@ void Sema::ArgumentDependentLookup(DeclarationName Name, bool Operator,
       if (isa<UsingShadowDecl>(D))
         D = cast<UsingShadowDecl>(D)->getTargetDecl();
 
-      // FIXME: canonical decls.
-      // See comment in AddArgumentDependentLookupCandidates().
-
       if (isa<FunctionDecl>(D)) {
         if (Operator &&
             !IsAcceptableNonMemberOperatorCandidate(cast<FunctionDecl>(D),
                                                     T1, T2, Context))
           continue;
-        Functions.insert(D);
-      } else if (isa<FunctionTemplateDecl>(D))
-        Functions.insert(D);
+      } else if (!isa<FunctionTemplateDecl>(D))
+        continue;
+
+      Result.insert(D);
     }
   }
 }

clang/lib/Sema/SemaOverload.cpp

@@ -4131,7 +4131,7 @@ Sema::AddArgumentDependentLookupCandidates(DeclarationName Name,
                        const TemplateArgumentListInfo *ExplicitTemplateArgs,
                                            OverloadCandidateSet& CandidateSet,
                                            bool PartialOverloading) {
-  ADLFunctionSet Functions;
+  ADLResult Fns;
 
   // FIXME: This approach for uniquing ADL results (and removing
   // redundant candidates from the set) relies on pointer-equality,
@@ -4141,22 +4141,21 @@ Sema::AddArgumentDependentLookupCandidates(DeclarationName Name,
   // we supposed to consider on ADL candidates, anyway?
 
   // FIXME: Pass in the explicit template arguments?
-  ArgumentDependentLookup(Name, Operator, Args, NumArgs, Functions);
+  ArgumentDependentLookup(Name, Operator, Args, NumArgs, Fns);
 
   // Erase all of the candidates we already knew about.
   for (OverloadCandidateSet::iterator Cand = CandidateSet.begin(),
                                    CandEnd = CandidateSet.end();
        Cand != CandEnd; ++Cand)
     if (Cand->Function) {
-      Functions.erase(Cand->Function);
+      Fns.erase(Cand->Function);
       if (FunctionTemplateDecl *FunTmpl = Cand->Function->getPrimaryTemplate())
-        Functions.erase(FunTmpl);
+        Fns.erase(FunTmpl);
     }
 
   // For each of the ADL candidates we found, add it to the overload
   // set.
-  for (ADLFunctionSet::iterator I = Functions.begin(),
-         E = Functions.end(); I != E; ++I) {
+  for (ADLResult::iterator I = Fns.begin(), E = Fns.end(); I != E; ++I) {
     if (FunctionDecl *FD = dyn_cast<FunctionDecl>(*I)) {
       if (ExplicitTemplateArgs)
         continue;

clang/test/SemaCXX/using-decl-1.cpp

@@ -77,3 +77,21 @@ namespace test0 {
     foo(*p); // expected-error {{no matching function for call to 'foo'}}
   }
 }
+
+// Redeclarations!
+namespace test1 {
+  namespace ns0 { struct Foo {}; }
+  namespace A { void foo(ns0::Foo *p, int y, int z); }
+  namespace ns2 { using A::foo; }
+  namespace ns1 { struct Bar : ns0::Foo {}; }
+  namespace A { void foo(ns0::Foo *p, int y, int z = 0); } // expected-note {{candidate}}
+  namespace ns1 { using A::foo; }
+  namespace ns2 { struct Baz : ns1::Bar {}; }
+  namespace A { void foo(ns0::Foo *p, int y = 0, int z); }
+
+  void test(ns2::Baz *p) {
+    foo(p, 0, 0); // okay!
+    foo(p, 0); // should be fine!
+    foo(p); // expected-error {{no matching function}}
+  }
+}