C++ doesn't really use "namespaces" for different kinds of names the same

way that C does.  Among other differences, elaborated type specifiers
are defined to skip "non-types", which, as you might imagine, does not
include typedefs.  Rework our use of IDNS masks to capture the semantics
of different kinds of declarations better, and remove most current lookup
filters.  Removing the last remaining filter is more complicated and will
happen in a separate patch.

Fixes PR 6885 as well some spectrum of unfiled bugs.



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

5 files changed, 126 insertions, 81 deletions

diff --git a/lib/AST/ASTImporter.cpp b/lib/AST/ASTImporter.cpp
index d3268030c9..a97536cc77 100644
--- a/lib/AST/ASTImporter.cpp
+++ b/lib/AST/ASTImporter.cpp
@@ -1438,7 +1438,7 @@ Decl *ASTNodeImporter::VisitNamespaceDecl(NamespaceDecl *D) {
     for (DeclContext::lookup_result Lookup = DC->lookup(Name);
          Lookup.first != Lookup.second; 
          ++Lookup.first) {
-      if (!(*Lookup.first)->isInIdentifierNamespace(Decl::IDNS_Ordinary))
+      if (!(*Lookup.first)->isInIdentifierNamespace(Decl::IDNS_Namespace))
         continue;
       
       if (NamespaceDecl *FoundNS = dyn_cast<NamespaceDecl>(*Lookup.first)) {
@@ -1451,7 +1451,7 @@ Decl *ASTNodeImporter::VisitNamespaceDecl(NamespaceDecl *D) {
     }
     
     if (!ConflictingDecls.empty()) {
-      Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Ordinary,
+      Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
                                          ConflictingDecls.data(), 
                                          ConflictingDecls.size());
     }
diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp
index 61d22b9d70..b5aec0c512 100644
--- a/lib/AST/DeclBase.cpp
+++ b/lib/AST/DeclBase.cpp
@@ -231,23 +231,28 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) {
     case CXXConstructor:
     case CXXDestructor:
     case CXXConversion:
-    case Typedef:
     case EnumConstant:
     case Var:
     case ImplicitParam:
     case ParmVar:
     case NonTypeTemplateParm:
     case ObjCMethod:
-    case ObjCInterface:
     case ObjCProperty:
-    case ObjCCompatibleAlias:
       return IDNS_Ordinary;
 
+    case ObjCCompatibleAlias:
+    case ObjCInterface:
+      return IDNS_Ordinary | IDNS_Type;
+
+    case Typedef:
+    case UnresolvedUsingTypename:
+    case TemplateTypeParm:
+      return IDNS_Ordinary | IDNS_Type;
+
     case UsingShadow:
       return 0; // we'll actually overwrite this later
 
     case UnresolvedUsingValue:
-    case UnresolvedUsingTypename:
       return IDNS_Ordinary | IDNS_Using;
 
     case Using:
@@ -264,15 +269,18 @@ unsigned Decl::getIdentifierNamespaceForKind(Kind DeclKind) {
     case Record:
     case CXXRecord:
     case Enum:
-    case TemplateTypeParm:
-      return IDNS_Tag;
+      return IDNS_Tag | IDNS_Type;
 
     case Namespace:
+    case NamespaceAlias:
+      return IDNS_Namespace;
+
     case FunctionTemplate:
+      return IDNS_Ordinary;
+
     case ClassTemplate:
     case TemplateTemplateParm:
-    case NamespaceAlias:
-      return IDNS_Tag | IDNS_Ordinary;
+      return IDNS_Ordinary | IDNS_Tag | IDNS_Type;
 
     // Never have names.
     case Friend:
diff --git a/lib/Sema/SemaCodeComplete.cpp b/lib/Sema/SemaCodeComplete.cpp
index e71f8c84eb..8be3da06ac 100644
--- a/lib/Sema/SemaCodeComplete.cpp
+++ b/lib/Sema/SemaCodeComplete.cpp
@@ -415,6 +415,9 @@ bool ResultBuilder::isInterestingDecl(NamedDecl *ND,
 
     return false;
   }
+
+  if (Filter == &ResultBuilder::IsNestedNameSpecifier)
+    AsNestedNameSpecifier = true;
   
   // ... then it must be interesting!
   return true;
@@ -504,7 +507,8 @@ void ResultBuilder::MaybeAddResult(Result R, DeclContext *CurContext) {
     }
     for (; I != IEnd; ++I) {
       // A tag declaration does not hide a non-tag declaration.
-      if (I->first->getIdentifierNamespace() == Decl::IDNS_Tag &&
+      if (I->first->getIdentifierNamespace()
+            == (Decl::IDNS_Tag | Decl::IDNS_Type) &&
           (IDNS & (Decl::IDNS_Member | Decl::IDNS_Ordinary | 
                    Decl::IDNS_ObjCProtocol)))
         continue;
@@ -629,7 +633,7 @@ void ResultBuilder::ExitScope() {
 bool ResultBuilder::IsOrdinaryName(NamedDecl *ND) const {
   unsigned IDNS = Decl::IDNS_Ordinary;
   if (SemaRef.getLangOptions().CPlusPlus)
-    IDNS |= Decl::IDNS_Tag;
+    IDNS |= Decl::IDNS_Tag | Decl::IDNS_Namespace;
   else if (SemaRef.getLangOptions().ObjC1 && isa<ObjCIvarDecl>(ND))
     return true;
 
@@ -641,7 +645,7 @@ bool ResultBuilder::IsOrdinaryName(NamedDecl *ND) const {
 bool ResultBuilder::IsOrdinaryNonValueName(NamedDecl *ND) const {
   unsigned IDNS = Decl::IDNS_Ordinary;
   if (SemaRef.getLangOptions().CPlusPlus)
-    IDNS |= Decl::IDNS_Tag;
+    IDNS |= Decl::IDNS_Tag | Decl::IDNS_Namespace;
   
   return (ND->getIdentifierNamespace() & IDNS) && 
     !isa<ValueDecl>(ND) && !isa<FunctionTemplateDecl>(ND);
@@ -2094,10 +2098,16 @@ void Sema::CodeCompleteTag(Scope *S, unsigned TagSpec) {
     return;
   }
   
-  ResultBuilder Results(*this, Filter);
-  Results.allowNestedNameSpecifiers();
+  ResultBuilder Results(*this);
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
+
+  // First pass: look for tags.
+  Results.setFilter(Filter);
   LookupVisibleDecls(S, LookupTagName, Consumer);
+
+  // Second pass: look for nested name specifiers.
+  Results.setFilter(&ResultBuilder::IsNestedNameSpecifier);
+  LookupVisibleDecls(S, LookupNestedNameSpecifierName, Consumer);
   
   HandleCodeCompleteResults(this, CodeCompleter, Results.data(),Results.size());
 }
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index c9001f1030..e0ca252b06 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -5000,7 +5000,8 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
       SearchDC = SearchDC->getEnclosingNamespaceContext();
     }
 
-    // In C++, look for a shadow friend decl.
+    // In C++, we need to do a redeclaration lookup to properly
+    // diagnose some problems.
     if (getLangOptions().CPlusPlus) {
       Previous.setRedeclarationKind(ForRedeclaration);
       LookupQualifiedName(Previous, SearchDC);
@@ -5009,6 +5010,26 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
 
   if (!Previous.empty()) {
     NamedDecl *PrevDecl = (*Previous.begin())->getUnderlyingDecl();
+
+    // It's okay to have a tag decl in the same scope as a typedef
+    // which shadows a tag decl in the same scope.  Finding this
+    // insanity with a redeclaration lookup can only actually happen
+    // in C++.
+    if (getLangOptions().CPlusPlus &&
+        TUK != TUK_Reference && TUK != TUK_Friend) {
+      if (TypedefDecl *TD = dyn_cast<TypedefDecl>(PrevDecl)) {
+        if (const TagType *TT = TD->getUnderlyingType()->getAs<TagType>()) {
+          TagDecl *Tag = TT->getDecl();
+          if (Tag->getDeclName() == Name &&
+              Tag->getDeclContext()->Equals(SearchDC)) {
+            PrevDecl = Tag;
+            Previous.clear();
+            Previous.addDecl(Tag);
+          }
+        }
+      }
+    }
+
     if (TagDecl *PrevTagDecl = dyn_cast<TagDecl>(PrevDecl)) {
       // If this is a use of a previous tag, or if the tag is already declared
       // in the same scope (so that the definition/declaration completes or
@@ -5100,23 +5121,61 @@ Sema::DeclPtrTy Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
       // If we get here, we're going to create a new Decl. If PrevDecl
       // is non-NULL, it's a definition of the tag declared by
       // PrevDecl. If it's NULL, we have a new definition.
+
+
+    // Otherwise, PrevDecl is not a tag, but was found with tag
+    // lookup.  This is only actually possible in C++, where a few
+    // things like templates still live in the tag namespace.
     } else {
-      // PrevDecl is a namespace, template, or anything else
-      // that lives in the IDNS_Tag identifier namespace.
-      if (TUK == TUK_Reference || TUK == TUK_Friend ||
-          isDeclInScope(PrevDecl, SearchDC, S)) {
-        // The tag name clashes with a namespace name, issue an error and
-        // recover by making this tag be anonymous.
+      assert(getLangOptions().CPlusPlus);
+
+      // Use a better diagnostic if an elaborated-type-specifier
+      // found the wrong kind of type on the first
+      // (non-redeclaration) lookup.
+      if ((TUK == TUK_Reference || TUK == TUK_Friend) &&
+          !Previous.isForRedeclaration()) {
+        unsigned Kind = 0;
+        if (isa<TypedefDecl>(PrevDecl)) Kind = 1;
+        else if (isa<ClassTemplateDecl>(PrevDecl)) Kind = 2;
+        Diag(NameLoc, diag::err_tag_reference_non_tag) << Kind;
+        Diag(PrevDecl->getLocation(), diag::note_declared_at);
+        Invalid = true;
+
+      // Otherwise, only diagnose if the declaration is in scope.
+      } else if (!isDeclInScope(PrevDecl, SearchDC, S)) {
+        // do nothing
+
+      // Diagnose implicit declarations introduced by elaborated types.
+      } else if (TUK == TUK_Reference || TUK == TUK_Friend) {
+        unsigned Kind = 0;
+        if (isa<TypedefDecl>(PrevDecl)) Kind = 1;
+        else if (isa<ClassTemplateDecl>(PrevDecl)) Kind = 2;
+        Diag(NameLoc, diag::err_tag_reference_conflict) << Kind;
+        Diag(PrevDecl->getLocation(), diag::note_previous_decl) << PrevDecl;
+        Invalid = true;
+
+      // Otherwise it's a declaration.  Call out a particularly common
+      // case here.
+      } else if (isa<TypedefDecl>(PrevDecl)) {
+        Diag(NameLoc, diag::err_tag_definition_of_typedef)
+          << Name
+          << cast<TypedefDecl>(PrevDecl)->getUnderlyingType();
+        Diag(PrevDecl->getLocation(), diag::note_previous_decl) << PrevDecl;
+        Invalid = true;
+
+      // Otherwise, diagnose.
+      } else {
+        // The tag name clashes with something else in the target scope,
+        // issue an error and recover by making this tag be anonymous.
         Diag(NameLoc, diag::err_redefinition_different_kind) << Name;
         Diag(PrevDecl->getLocation(), diag::note_previous_definition);
         Name = 0;
-        Previous.clear();
         Invalid = true;
-      } else {
-        // The existing declaration isn't relevant to us; we're in a
-        // new scope, so clear out the previous declaration.
-        Previous.clear();
       }
+
+      // The existing declaration isn't relevant to us; we're in a
+      // new scope, so clear out the previous declaration.
+      Previous.clear();
     }
   }
 
@@ -5187,28 +5246,6 @@ CreateNewDecl:
       New->addAttr(::new (Context) PragmaPackAttr(Alignment * 8));
   }
 
-  if (getLangOptions().CPlusPlus && SS.isEmpty() && Name && !Invalid) {
-    // C++ [dcl.typedef]p3:
-    //   [...] Similarly, in a given scope, a class or enumeration
-    //   shall not be declared with the same name as a typedef-name
-    //   that is declared in that scope and refers to a type other
-    //   than the class or enumeration itself.
-    LookupResult Lookup(*this, Name, NameLoc, LookupOrdinaryName,
-                        ForRedeclaration);
-    LookupName(Lookup, S);
-    TypedefDecl *PrevTypedef = Lookup.getAsSingle<TypedefDecl>();
-    NamedDecl *PrevTypedefNamed = PrevTypedef;
-    if (PrevTypedef && isDeclInScope(PrevTypedefNamed, SearchDC, S) &&
-        Context.getCanonicalType(Context.getTypeDeclType(PrevTypedef)) !=
-          Context.getCanonicalType(Context.getTypeDeclType(New))) {
-      Diag(Loc, diag::err_tag_definition_of_typedef)
-        << Context.getTypeDeclType(New)
-        << PrevTypedef->getUnderlyingType();
-      Diag(PrevTypedef->getLocation(), diag::note_previous_definition);
-      Invalid = true;
-    }
-  }
-
   // If this is a specialization of a member class (of a class template),
   // check the specialization.
   if (isExplicitSpecialization && CheckMemberSpecialization(New, Previous))
diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp
index 8de0c40fb7..445d08e666 100644
--- a/lib/Sema/SemaLookup.cpp
+++ b/lib/Sema/SemaLookup.cpp
@@ -202,26 +202,6 @@ static bool IsAcceptableOperatorName(NamedDecl *D, unsigned IDNS) {
     !D->getDeclContext()->isRecord();
 }
 
-static bool IsAcceptableNestedNameSpecifierName(NamedDecl *D, unsigned IDNS) {
-  // This lookup ignores everything that isn't a type.
-
-  // This is a fast check for the far most common case.
-  if (D->isInIdentifierNamespace(Decl::IDNS_Tag))
-    return true;
-
-  if (isa<UsingShadowDecl>(D))
-    D = cast<UsingShadowDecl>(D)->getTargetDecl();
-
-  return isa<TypeDecl>(D);
-}
-
-static bool IsAcceptableNamespaceName(NamedDecl *D, unsigned IDNS) {
-  // We don't need to look through using decls here because
-  // using decls aren't allowed to name namespaces.
-
-  return isa<NamespaceDecl>(D) || isa<NamespaceAliasDecl>(D);
-}
-
 /// Gets the default result filter for the given lookup.
 static inline
 LookupResult::ResultFilter getResultFilter(Sema::LookupNameKind NameKind) {
@@ -232,16 +212,12 @@ LookupResult::ResultFilter getResultFilter(Sema::LookupNameKind NameKind) {
   case Sema::LookupRedeclarationWithLinkage: // FIXME: check linkage, scoping
   case Sema::LookupUsingDeclName:
   case Sema::LookupObjCProtocolName:
+  case Sema::LookupNestedNameSpecifierName:
+  case Sema::LookupNamespaceName:
     return &IsAcceptableIDNS;
 
   case Sema::LookupOperatorName:
     return &IsAcceptableOperatorName;
-
-  case Sema::LookupNestedNameSpecifierName:
-    return &IsAcceptableNestedNameSpecifierName;
-
-  case Sema::LookupNamespaceName:
-    return &IsAcceptableNamespaceName;
   }
 
   llvm_unreachable("unkknown lookup kind");
@@ -260,15 +236,25 @@ static inline unsigned getIDNS(Sema::LookupNameKind NameKind,
   case Sema::LookupRedeclarationWithLinkage:
     IDNS = Decl::IDNS_Ordinary;
     if (CPlusPlus) {
-      IDNS |= Decl::IDNS_Tag | Decl::IDNS_Member;
+      IDNS |= Decl::IDNS_Tag | Decl::IDNS_Member | Decl::IDNS_Namespace;
       if (Redeclaration) IDNS |= Decl::IDNS_TagFriend | Decl::IDNS_OrdinaryFriend;
     }
     break;
 
   case Sema::LookupTagName:
-    IDNS = Decl::IDNS_Tag;
-    if (CPlusPlus && Redeclaration)
-      IDNS |= Decl::IDNS_TagFriend;
+    if (CPlusPlus) {
+      IDNS = Decl::IDNS_Type;
+
+      // When looking for a redeclaration of a tag name, we add:
+      // 1) TagFriend to find undeclared friend decls
+      // 2) Namespace because they can't "overload" with tag decls.
+      // 3) Tag because it includes class templates, which can't
+      //    "overload" with tag decls.
+      if (Redeclaration)
+        IDNS |= Decl::IDNS_Tag | Decl::IDNS_TagFriend | Decl::IDNS_Namespace;
+    } else {
+      IDNS = Decl::IDNS_Tag;
+    }
     break;
 
   case Sema::LookupMemberName:
@@ -278,8 +264,11 @@ static inline unsigned getIDNS(Sema::LookupNameKind NameKind,
     break;
 
   case Sema::LookupNestedNameSpecifierName:
+    IDNS = Decl::IDNS_Type | Decl::IDNS_Namespace;
+    break;
+
   case Sema::LookupNamespaceName:
-    IDNS = Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Member;
+    IDNS = Decl::IDNS_Namespace;
     break;
 
   case Sema::LookupUsingDeclName:
@@ -2134,7 +2123,8 @@ NamedDecl *VisibleDeclsRecord::checkHidden(NamedDecl *ND) {
                                IEnd = Pos->second.end();
          I != IEnd; ++I) {
       // A tag declaration does not hide a non-tag declaration.
-      if ((*I)->getIdentifierNamespace() == Decl::IDNS_Tag &&
+      if ((*I)->getIdentifierNamespace()
+            == (Decl::IDNS_Tag|Decl::IDNS_Type) &&
           (IDNS & (Decl::IDNS_Member | Decl::IDNS_Ordinary | 
                    Decl::IDNS_ObjCProtocol)))
         continue;