r264323 - Change ADL to produce lookup results in a deterministic order. This fixes some

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Mar 24 14:12:22 2016
New Revision: 264323

URL: http://llvm.org/viewvc/llvm-project?rev=264323&view=rev
Log:
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.

Added:
    cfe/trunk/test/SemaCXX/diagnostic-order.cpp
Modified:
    cfe/trunk/include/clang/Sema/Lookup.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=264323&r1=264322&r2=264323&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Thu Mar 24 14:12:22 2016
@@ -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()); }
 };
 
 }

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=264323&r1=264322&r2=264323&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 24 14:12:22 2016
@@ -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,

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=264323&r1=264322&r2=264323&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Mar 24 14:12:22 2016
@@ -2446,7 +2446,7 @@ addAssociatedClassesAndNamespaces(Associ
   // 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(Associ
       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);

Added: cfe/trunk/test/SemaCXX/diagnostic-order.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/diagnostic-order.cpp?rev=264323&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/diagnostic-order.cpp (added)
+++ cfe/trunk/test/SemaCXX/diagnostic-order.cpp Thu Mar 24 14:12:22 2016
@@ -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'