From: Richard Smith <richard-llvm@metafoo.co.uk>
Date: Thu, 24 Mar 2016 19:12:22 +0000 (+0000)
Subject: Change ADL to produce lookup results in a deterministic order. This fixes some
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6ab9f3a1f7015b212ed28d5caa5d926e761e4426;p=clang

Change ADL to produce lookup results in a deterministic order. This fixes some
rare issues with nondeterministic diagnostic order, and some very common issues
with nondeterministic module builds.


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

diff --git a/include/clang/Sema/Lookup.h b/include/clang/Sema/Lookup.h
index 81fc5a5e15..2ed9548b59 100644
--- a/include/clang/Sema/Lookup.h
+++ b/include/clang/Sema/Lookup.h
@@ -769,7 +769,13 @@ public:
 class ADLResult {
 private:
   /// A map from canonical decls to the 'most recent' decl.
-  llvm::DenseMap<NamedDecl*, NamedDecl*> Decls;
+  llvm::MapVector<NamedDecl*, NamedDecl*> Decls;
+
+  struct select_second {
+    NamedDecl *operator()(std::pair<NamedDecl*, NamedDecl*> P) const {
+      return P.second;
+    }
+  };
 
 public:
   /// Adds a new ADL candidate to this map.
@@ -780,23 +786,11 @@ public:
     Decls.erase(cast<NamedDecl>(D->getCanonicalDecl()));
   }
 
-  class iterator
-      : public llvm::iterator_adaptor_base<
-            iterator, llvm::DenseMap<NamedDecl *, NamedDecl *>::iterator,
-            std::forward_iterator_tag, NamedDecl *> {
-    friend class ADLResult;
-
-    iterator(llvm::DenseMap<NamedDecl *, NamedDecl *>::iterator Iter)
-        : iterator_adaptor_base(std::move(Iter)) {}
-
-  public:
-    iterator() {}
-
-    value_type operator*() const { return I->second; }
-  };
+  typedef llvm::mapped_iterator<decltype(Decls)::iterator, select_second>
+      iterator;
 
-  iterator begin() { return iterator(Decls.begin()); }
-  iterator end() { return iterator(Decls.end()); }
+  iterator begin() { return iterator(Decls.begin(), select_second()); }
+  iterator end() { return iterator(Decls.end(), select_second()); }
 };
 
 }
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index 34fedd2160..377fb182a4 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -2385,8 +2385,8 @@ public:
 
   // Members have to be NamespaceDecl* or TranslationUnitDecl*.
   // TODO: make this is a typesafe union.
-  typedef llvm::SmallPtrSet<DeclContext   *, 16> AssociatedNamespaceSet;
-  typedef llvm::SmallPtrSet<CXXRecordDecl *, 16> AssociatedClassSet;
+  typedef llvm::SmallSetVector<DeclContext   *, 16> AssociatedNamespaceSet;
+  typedef llvm::SmallSetVector<CXXRecordDecl *, 16> AssociatedClassSet;
 
   void AddOverloadCandidate(FunctionDecl *Function,
                             DeclAccessPair FoundDecl,
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 0037933625..08133d6889 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -2446,7 +2446,7 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
   // 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).second)
+  if (!Result.Classes.insert(Class))
     return;
 
   // -- If T is a template-id, its associated namespaces and classes are
@@ -2496,7 +2496,7 @@ addAssociatedClassesAndNamespaces(AssociatedLookup &Result,
       if (!BaseType)
         continue;
       CXXRecordDecl *BaseDecl = cast<CXXRecordDecl>(BaseType->getDecl());
-      if (Result.Classes.insert(BaseDecl).second) {
+      if (Result.Classes.insert(BaseDecl)) {
         // Find the associated namespace for this base class.
         DeclContext *BaseCtx = BaseDecl->getDeclContext();
         CollectEnclosingNamespace(Result.Namespaces, BaseCtx);
diff --git a/test/SemaCXX/diagnostic-order.cpp b/test/SemaCXX/diagnostic-order.cpp
new file mode 100644
index 0000000000..f0899018f7
--- /dev/null
+++ b/test/SemaCXX/diagnostic-order.cpp
@@ -0,0 +1,20 @@
+// RUN: not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+
+// Ensure that the diagnostics we produce for this situation appear in a
+// deterministic order. This requires ADL to provide lookup results in a
+// deterministic order.
+template<typename T> struct Error { typedef typename T::error error; };
+struct X { template<typename T> friend typename Error<T>::error f(X, T); };
+struct Y { template<typename T> friend typename Error<T>::error f(T, Y); };
+
+void g() {
+  f(X(), Y());
+}
+
+// We don't really care which order these two diagnostics appear (although the
+// order below is source order, which seems best). The crucial fact is that
+// there is one single order that is stable across multiple runs of clang.
+//
+// CHECK: no type named 'error' in 'Y'
+// CHECK: no type named 'error' in 'X'
+// CHECK: no matching function for call to 'f'