Remove more redundant lookups. Add a new "all_lookups_iterator" which provides

a view over the contents of a DeclContext without exposing the implementation
details of the StoredDeclsMap. Use this in LookupVisibleDecls to find the
visible declarations. Fixes PR12339!


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

7 files changed, 116 insertions, 26 deletions

diff --git a/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h
index 4c1fefef50..4c675aed8f 100644
--- a/include/clang/AST/DeclBase.h
+++ b/include/clang/AST/DeclBase.h
@@ -1438,6 +1438,14 @@ public:
   /// replaced with D.
   void makeDeclVisibleInContext(NamedDecl *D);
 
+  /// all_lookups_iterator - An iterator that provides a view over the results
+  /// of looking up every possible name.
+  class all_lookups_iterator;
+
+  all_lookups_iterator lookups_begin() const;
+
+  all_lookups_iterator lookups_end() const;
+
   /// udir_iterator - Iterates through the using-directives stored
   /// within this context.
   typedef UsingDirectiveDecl * const * udir_iterator;
diff --git a/include/clang/AST/DeclLookups.h b/include/clang/AST/DeclLookups.h
new file mode 100644
index 0000000000..66d190f429
--- /dev/null
+++ b/include/clang/AST/DeclLookups.h
@@ -0,0 +1,84 @@
+//===-- DeclLookups.h - Low-level interface to all names in a DC-*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines DeclContext::all_lookups_iterator.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_DECLLOOKUPS_H
+#define LLVM_CLANG_AST_DECLLOOKUPS_H
+
+#include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclContextInternals.h"
+#include "clang/AST/DeclarationName.h"
+
+namespace clang {
+
+/// all_lookups_iterator - An iterator that provides a view over the results
+/// of looking up every possible name.
+class DeclContext::all_lookups_iterator {
+  StoredDeclsMap::iterator It, End;
+public:
+  typedef lookup_result             value_type;
+  typedef lookup_result             reference;
+  typedef lookup_result             pointer;
+  typedef std::forward_iterator_tag iterator_category;
+  typedef std::ptrdiff_t            difference_type;
+
+  all_lookups_iterator() {}
+  all_lookups_iterator(StoredDeclsMap::iterator It,
+                       StoredDeclsMap::iterator End)
+      : It(It), End(End) {}
+
+  reference operator*() const { return It->second.getLookupResult(); }
+  pointer operator->() const { return It->second.getLookupResult(); }
+
+  all_lookups_iterator& operator++() {
+    // Filter out using directives. They don't belong as results from name
+    // lookup anyways, except as an implementation detail. Users of the API
+    // should not expect to get them (or worse, rely on it).
+    do {
+      ++It;
+    } while (It != End &&
+             It->first == DeclarationName::getUsingDirectiveName());
+             
+    return *this;
+  }
+
+  all_lookups_iterator operator++(int) {
+    all_lookups_iterator tmp(*this);
+    ++(*this);
+    return tmp;
+  }
+
+  friend bool operator==(all_lookups_iterator x, all_lookups_iterator y) {
+    return x.It == y.It;
+  }
+  friend bool operator!=(all_lookups_iterator x, all_lookups_iterator y) {
+    return x.It != y.It;
+  }
+};
+
+DeclContext::all_lookups_iterator DeclContext::lookups_begin() const {
+  DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
+  if (StoredDeclsMap *Map = Primary->buildLookup())
+    return all_lookups_iterator(Map->begin(), Map->end());
+  return all_lookups_iterator();
+}
+
+DeclContext::all_lookups_iterator DeclContext::lookups_end() const {
+  DeclContext *Primary = const_cast<DeclContext*>(this)->getPrimaryContext();
+  if (StoredDeclsMap *Map = Primary->buildLookup())
+    return all_lookups_iterator(Map->end(), Map->end());
+  return all_lookups_iterator();
+}
+
+} // end namespace clang
+
+#endif
diff --git a/lib/Sema/SemaCodeComplete.cpp b/lib/Sema/SemaCodeComplete.cpp
index 3fd66ec042..c09cd177be 100644
--- a/lib/Sema/SemaCodeComplete.cpp
+++ b/lib/Sema/SemaCodeComplete.cpp
@@ -514,14 +514,6 @@ bool ResultBuilder::isInterestingDecl(NamedDecl *ND,
         return false;
     }
   }
-   
-  // Skip out-of-line declarations and definitions.
-  // NOTE: Unless it's an Objective-C property, method, or ivar, where
-  // the contexts can be messy.
-  if (!ND->getDeclContext()->Equals(ND->getLexicalDeclContext()) &&
-      !(isa<ObjCPropertyDecl>(ND) || isa<ObjCIvarDecl>(ND) ||
-        isa<ObjCMethodDecl>(ND)))
-    return false;
 
   if (Filter == &ResultBuilder::IsNestedNameSpecifier ||
       ((isa<NamespaceDecl>(ND) || isa<NamespaceAliasDecl>(ND)) &&
@@ -904,7 +896,7 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
 
   if (Hiding && CheckHiddenResult(R, CurContext, Hiding))
     return;
-      
+
   // Make sure that any given declaration only shows up in the result set once.
   if (!AllDeclsFound.insert(R.Declaration->getCanonicalDecl()))
     return;
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 9fb248ed5b..966eb90306 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -25,6 +25,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclLookups.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
@@ -2875,26 +2876,16 @@ static void LookupVisibleDecls(DeclContext *Ctx, LookupResult &Result,
     Result.getSema().ForceDeclarationOfImplicitMembers(Class);
 
   // Enumerate all of the results in this context.
-  llvm::SmallVector<DeclContext *, 2> Contexts;
-  Ctx->collectAllContexts(Contexts);
-  for (unsigned I = 0, N = Contexts.size(); I != N; ++I) {
-    DeclContext *CurCtx = Contexts[I];
-    for (DeclContext::decl_iterator D = CurCtx->decls_begin(),
-                                 DEnd = CurCtx->decls_end();
-         D != DEnd; ++D) {
-      if (NamedDecl *ND = dyn_cast<NamedDecl>(*D)) {
+  for (DeclContext::all_lookups_iterator L = Ctx->lookups_begin(),
+                                      LEnd = Ctx->lookups_end();
+       L != LEnd; ++L) {
+    for (DeclContext::lookup_result R = *L; R.first != R.second; ++R.first) {
+      if (NamedDecl *ND = dyn_cast<NamedDecl>(*R.first)) {
         if ((ND = Result.getAcceptableDecl(ND))) {
           Consumer.FoundDecl(ND, Visited.checkHidden(ND), Ctx, InBaseClass);
           Visited.add(ND);
         }
       }
-      
-      // Visit transparent contexts and inline namespaces inside this context.
-      if (DeclContext *InnerCtx = dyn_cast<DeclContext>(*D)) {
-        if (InnerCtx->isTransparentContext() || InnerCtx->isInlineNamespace())
-          LookupVisibleDecls(InnerCtx, Result, QualifiedNameLookup, InBaseClass,
-                             Consumer, Visited);
-      }
     }
   }
 
diff --git a/test/SemaCXX/function-redecl.cpp b/test/SemaCXX/function-redecl.cpp
index 0d9ecf334e..0eb109d263 100644
--- a/test/SemaCXX/function-redecl.cpp
+++ b/test/SemaCXX/function-redecl.cpp
@@ -76,9 +76,12 @@ class Crash {
   void GetCart(int count) const;
 };
 // This out-of-line definition was fine...
-void Crash::cart(int count) const {} // expected-error {{out-of-line definition of 'cart' does not match any declaration in 'Crash'}}
+void Crash::cart(int count) const {} // expected-error {{out-of-line definition of 'cart' does not match any declaration in 'Crash'}} \
+                                     // expected-note {{'cart' declared here}} \
+                                     // expected-note {{previous definition is here}}
 // ...while this one crashed clang
-void Crash::chart(int count) const {} // expected-error {{out-of-line definition of 'chart' does not match any declaration in 'Crash'}}
+void Crash::chart(int count) const {} // expected-error {{out-of-line definition of 'chart' does not match any declaration in 'Crash'; did you mean 'cart'?}} \
+                                      // expected-error {{redefinition of 'cart'}}
 
 class TestConst {
  public:
diff --git a/test/SemaCXX/qualified-id-lookup.cpp b/test/SemaCXX/qualified-id-lookup.cpp
index 3cd6e1894b..d65a4684e6 100644
--- a/test/SemaCXX/qualified-id-lookup.cpp
+++ b/test/SemaCXX/qualified-id-lookup.cpp
@@ -146,3 +146,8 @@ namespace PR6830 {
     Z(foo::X()).Work();
   }
 }
+
+namespace pr12339 {
+  extern "C" void i;
+  pr12339::FOO  // expected-error{{no type named 'FOO' in namespace 'pr12339'}}
+}  // expected-error{{expected unqualified-id}}
diff --git a/test/SemaCXX/typo-correction.cpp b/test/SemaCXX/typo-correction.cpp
index 6fe31c7c33..b1e8d91d45 100644
--- a/test/SemaCXX/typo-correction.cpp
+++ b/test/SemaCXX/typo-correction.cpp
@@ -183,3 +183,10 @@ class ClashTool {
         getExprAs<ConstructExpr>(); // expected-error{{use of undeclared identifier 'ConstructExpr'; did you mean 'clash::ConstructExpr'?}}
   }
 };
+
+namespace test1 {
+  struct S {
+    struct Foobar *f;  // expected-note{{'Foobar' declared here}}
+  };
+  test1::FooBar *b;  // expected-error{{no type named 'FooBar' in namespace 'test1'; did you mean 'Foobar'?}}
+}