Richard Smith wrote:
On Fri, Oct 5, 2012 at 2:21 AM, Nick Lewycky <[email protected]
<mailto:[email protected]>> wrote:

    This patch changes UsingDirectives to be derived from Decl instead
    of NamedDecl, and consequently removes the CXXUsingDirective
    DeclarationName.

    The way using directives are stored with this patch is to have a
    single pointer on each UsingDirectiveDecl that points to the
    previous declaration, and a pointer on TranslationUnit and Namespace
    DeclContexts that point to the last one in that chain. We don't need
    such pointers on other DeclContexts because they either can't hold
    using-directives or the lookup will be handled by Scope.

    Please review!


This looks good to me. Have you tested the patch with chained PCH (or
modules)?

No, and in doing so I found all sorts of problems. This updated patch teaches PCH to actually serialize these things. Namespaces need to serialize LastUsingDirective, and I'm doing something a little inefficient with the translation-unit wide ones, marking them all "external", which is technically inefficient in the PCH format but not harmful.

I also found a case where we would put a UsingDirectiveDecl inside a LinkageSpecDecl instead of the NamespaceDecl or TranslationUnitDecl and changed the code for that too. This means we build a different AST, but I think there's no difference in behaviour. (Otherwise, we would need to add a LastUsingDirective field to LinkageSpecDecls as well, and I don't think we need that.)

Please review!

Nick
Index: tools/libclang/CursorVisitor.h
===================================================================
--- tools/libclang/CursorVisitor.h	(revision 165388)
+++ tools/libclang/CursorVisitor.h	(working copy)
@@ -1,4 +1,4 @@
-//===- CursorVisitor.h - CursorVisitor interface --------------------------===//
+//===- CursorVisitor.h - CursorVisitor interface ----------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
Index: tools/libclang/CIndex.cpp
===================================================================
--- tools/libclang/CIndex.cpp	(revision 165388)
+++ tools/libclang/CIndex.cpp	(working copy)
@@ -1152,7 +1152,6 @@
   case clang::DeclarationName::Identifier:
   case clang::DeclarationName::CXXLiteralOperatorName:
   case clang::DeclarationName::CXXOperatorName:
-  case clang::DeclarationName::CXXUsingDirective:
     return false;
       
   case clang::DeclarationName::CXXConstructorName:
@@ -3063,9 +3062,6 @@
     // ObjCCategoryImplDecl returns the category name.
     return createCXString(CIMP->getIdentifier()->getNameStart());
 
-  if (isa<UsingDirectiveDecl>(D))
-    return createCXString("");
-  
   SmallString<1024> S;
   llvm::raw_svector_ostream os(S);
   ND->printName(os);
Index: tools/libclang/IndexingContext.h
===================================================================
--- tools/libclang/IndexingContext.h	(revision 165388)
+++ tools/libclang/IndexingContext.h	(working copy)
@@ -1,4 +1,4 @@
-//===- IndexingContext.h - Higher level API functions ------------------------===//
+//===- IndexingContext.h - Higher level API functions -----------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
Index: tools/libclang/RecursiveASTVisitor.h
===================================================================
--- tools/libclang/RecursiveASTVisitor.h	(revision 165388)
+++ tools/libclang/RecursiveASTVisitor.h	(working copy)
@@ -633,7 +633,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     break;
   }
 
Index: tools/libclang/IndexDecl.cpp
===================================================================
--- tools/libclang/IndexDecl.cpp	(revision 165388)
+++ tools/libclang/IndexDecl.cpp	(working copy)
@@ -263,9 +263,10 @@
     // FIXME: Parent for the following is CXIdxEntity_Unexposed with no USR,
     // we should do better.
 
-    IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), D);
+    IndexCtx.indexNestedNameSpecifierLoc(D->getQualifierLoc(), 0,
+                                         D->getLexicalDeclContext());
     IndexCtx.handleReference(D->getNominatedNamespaceAsWritten(),
-                             D->getLocation(), D, D->getLexicalDeclContext());
+                             D->getLocation(), 0, D->getLexicalDeclContext());
     return true;
   }
 
Index: tools/libclang/CIndexUSRs.cpp
===================================================================
--- tools/libclang/CIndexUSRs.cpp	(revision 165388)
+++ tools/libclang/CIndexUSRs.cpp	(working copy)
@@ -88,9 +88,6 @@
   void VisitLinkageSpecDecl(LinkageSpecDecl *D) {
     IgnoreResults = true;
   }
-  void VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
-    IgnoreResults = true;
-  }
   void VisitUsingDecl(UsingDecl *D) { 
     IgnoreResults = true;
   }
Index: tools/libclang/IndexingContext.cpp
===================================================================
--- tools/libclang/IndexingContext.cpp	(revision 165388)
+++ tools/libclang/IndexingContext.cpp	(working copy)
@@ -1,4 +1,4 @@
-//===- CIndexHigh.cpp - Higher level API functions ------------------------===//
+//===- IndexingContext.cpp - Higher level API functions -------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h	(revision 165388)
+++ lib/Sema/TreeTransform.h	(working copy)
@@ -2783,7 +2783,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     return NameInfo;
 
   case DeclarationName::CXXConstructorName:
Index: lib/Sema/SemaLookup.cpp
===================================================================
--- lib/Sema/SemaLookup.cpp	(revision 165388)
+++ lib/Sema/SemaLookup.cpp	(working copy)
@@ -152,23 +152,24 @@
     // by its using directives, transitively) as if they appeared in
     // the given effective context.
     void addUsingDirectives(DeclContext *DC, DeclContext *EffectiveDC) {
-      SmallVector<DeclContext*,4> queue;
+      SmallVector<DeclContext*, 4> queue;
       while (true) {
-        DeclContext::udir_iterator I, End;
-        for (llvm::tie(I, End) = DC->getUsingDirectives(); I != End; ++I) {
-          UsingDirectiveDecl *UD = *I;
-          DeclContext *NS = UD->getNominatedNamespace();
-          if (visited.insert(NS)) {
-            addUsingDirective(UD, EffectiveDC);
-            queue.push_back(NS);
+        if (DC->isFileContext()) {
+          DeclContext::udir_iterator I, End;
+          for (llvm::tie(I, End) = DC->getUsingDirectives(); I != End; ++I) {
+            UsingDirectiveDecl *UD = *I;
+            DeclContext *NS = UD->getNominatedNamespace();
+            if (visited.insert(NS)) {
+              addUsingDirective(UD, EffectiveDC);
+              queue.push_back(NS);
+            }
           }
         }
 
         if (queue.empty())
           return;
 
-        DC = queue.back();
-        queue.pop_back();
+        DC = queue.pop_back_val();
       }
     }
 
@@ -1338,8 +1339,7 @@
 
 /// \brief Callback that looks for any member of a class with the given name.
 static bool LookupAnyMember(const CXXBaseSpecifier *Specifier,
-                            CXXBasePath &Path,
-                            void *Name) {
+                            CXXBasePath &Path, void *Name) {
   RecordDecl *BaseRecord = Specifier->getType()->getAs<RecordType>()->getDecl();
 
   DeclarationName N = DeclarationName::getFromOpaquePtr(Name);
@@ -2864,7 +2864,7 @@
   }
 
   // Traverse using directives for qualified name lookup.
-  if (QualifiedNameLookup) {
+  if (QualifiedNameLookup && Ctx->isFileContext()) {
     ShadowContextRAII Shadow(Visited);
     DeclContext::udir_iterator I, E;
     for (llvm::tie(I, E) = Ctx->getUsingDirectives(); I != E; ++I) {
Index: lib/Sema/SemaTemplateVariadic.cpp
===================================================================
--- lib/Sema/SemaTemplateVariadic.cpp	(revision 165388)
+++ lib/Sema/SemaTemplateVariadic.cpp	(working copy)
@@ -306,7 +306,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     return false;
 
   case DeclarationName::CXXConstructorName:
Index: lib/Sema/SemaCodeComplete.cpp
===================================================================
--- lib/Sema/SemaCodeComplete.cpp	(revision 165388)
+++ lib/Sema/SemaCodeComplete.cpp	(working copy)
@@ -2422,7 +2422,6 @@
                       Result.getAllocator().CopyString(ND->getNameAsString()));
     break;
       
-  case DeclarationName::CXXUsingDirective:
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
Index: lib/Sema/SemaDeclCXX.cpp
===================================================================
--- lib/Sema/SemaDeclCXX.cpp	(revision 165388)
+++ lib/Sema/SemaDeclCXX.cpp	(working copy)
@@ -5574,15 +5574,15 @@
 
     if (!PrevNS) {
       UsingDirectiveDecl* UD
-        = UsingDirectiveDecl::Create(Context, CurContext,
+        = UsingDirectiveDecl::Create(Context, Parent,
                                      /* 'using' */ LBrace,
                                      /* 'namespace' */ SourceLocation(),
                                      /* qualifier */ NestedNameSpecifierLoc(),
                                      /* identifier */ SourceLocation(),
                                      Namespc,
-                                     /* Ancestor */ CurContext);
+                                     /* Ancestor */ Parent);
       UD->setImplicit();
-      CurContext->addDecl(UD);
+      Parent->addDecl(UD);
     }
   }
 
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp	(revision 165388)
+++ lib/Serialization/ASTReaderDecl.cpp	(working copy)
@@ -1,4 +1,4 @@
-//===--- ASTReaderDecl.cpp - Decl Deserialization ---------------*- C++ -*-===//
+//===--- ASTReaderDecl.cpp - Decl Deserialization -------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -351,7 +351,7 @@
     DeclContext *SemaDC = ReadDeclAs<DeclContext>(Record, Idx);
     DeclContext *LexicalDC = ReadDeclAs<DeclContext>(Record, Idx);
     // Avoid calling setLexicalDeclContext() directly because it uses
-    // Decl::getASTContext() internally which is unsafe during derialization.
+    // Decl::getASTContext() internally which is unsafe during deserialization.
     D->setDeclContextsImpl(SemaDC, LexicalDC, Reader.getContext());
   }
   D->setLocation(Reader.ReadSourceLocation(F, RawLocation));
@@ -1020,6 +1020,9 @@
     // been deserialized.
     D->AnonOrFirstNamespaceAndInline.setPointer(D->getFirstDeclaration());
   }
+
+  NamespaceDecl *Primary = cast<NamespaceDecl>(D->getPrimaryContext());
+  Primary->LastUsingDirective = ReadDeclAs<UsingDirectiveDecl>(Record, Idx);
 }
 
 void ASTDeclReader::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
@@ -1051,12 +1054,19 @@
 }
 
 void ASTDeclReader::VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
-  VisitNamedDecl(D);
+  VisitDecl(D);
   D->UsingLoc = ReadSourceLocation(Record, Idx);
   D->NamespaceLoc = ReadSourceLocation(Record, Idx);
   D->QualifierLoc = Reader.ReadNestedNameSpecifierLoc(F, Record, Idx);
   D->NominatedNamespace = ReadDeclAs<NamedDecl>(Record, Idx);
   D->CommonAncestor = ReadDeclAs<DeclContext>(Record, Idx);
+  D->Prev = ReadDeclAs<UsingDirectiveDecl>(Record, Idx);
+  if (Decl *DC = ReadDecl(Record, Idx)) {
+    if (NamespaceDecl *N = dyn_cast<NamespaceDecl>(DC))
+      N->LastUsingDirective = D;
+    else
+      cast<TranslationUnitDecl>(DC)->LastUsingDirective = D;
+  }
 }
 
 void ASTDeclReader::VisitUnresolvedUsingValueDecl(UnresolvedUsingValueDecl *D) {
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp	(revision 165388)
+++ lib/Serialization/ASTReader.cpp	(working copy)
@@ -1,4 +1,4 @@
-//===--- ASTReader.cpp - AST File Reader ------------------------*- C++ -*-===//
+//===--- ASTReader.cpp - AST File Reader ----------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -633,7 +633,6 @@
   case DeclarationName::CXXConstructorName:
   case DeclarationName::CXXDestructorName:
   case DeclarationName::CXXConversionFunctionName:
-  case DeclarationName::CXXUsingDirective:
     break;
   }
 
@@ -663,7 +662,6 @@
   case DeclarationName::CXXConstructorName:
   case DeclarationName::CXXDestructorName:
   case DeclarationName::CXXConversionFunctionName:
-  case DeclarationName::CXXUsingDirective:
     Key.Data = 0;
     break;
   }
@@ -705,7 +703,6 @@
   case DeclarationName::CXXConstructorName:
   case DeclarationName::CXXDestructorName:
   case DeclarationName::CXXConversionFunctionName:
-  case DeclarationName::CXXUsingDirective:
     Key.Data = 0;
     break;
   }
@@ -5858,9 +5855,6 @@
   case DeclarationName::CXXLiteralOperatorName:
     return Context.DeclarationNames.getCXXLiteralOperatorName(
                                        GetIdentifierInfo(F, Record, Idx));
-
-  case DeclarationName::CXXUsingDirective:
-    return DeclarationName::getUsingDirectiveName();
   }
 
   llvm_unreachable("Invalid NameKind!");
@@ -5893,7 +5887,6 @@
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
-  case DeclarationName::CXXUsingDirective:
     break;
   }
 }
Index: lib/Serialization/ASTWriterDecl.cpp
===================================================================
--- lib/Serialization/ASTWriterDecl.cpp	(revision 165388)
+++ lib/Serialization/ASTWriterDecl.cpp	(working copy)
@@ -131,7 +131,7 @@
   // Source locations require array (variable-length) abbreviations.  The
   // abbreviation infrastructure requires that arrays are encoded last, so
   // we handle it here in the case of those classes derived from DeclaratorDecl
-  if (DeclaratorDecl *DD = dyn_cast<DeclaratorDecl>(D)){
+  if (DeclaratorDecl *DD = dyn_cast<DeclaratorDecl>(D)) {
     Writer.AddTypeSourceInfo(DD->getTypeSourceInfo(), Record);
   }
 
@@ -860,6 +860,9 @@
       Writer.AddDeclRef(D, Record);
     }
   }
+
+  NamespaceDecl *Primary = cast<NamespaceDecl>(D->getPrimaryContext());
+  Writer.AddDeclRef(Primary->LastUsingDirective, Record);
 }
 
 void ASTDeclWriter::VisitNamespaceAliasDecl(NamespaceAliasDecl *D) {
@@ -891,12 +894,26 @@
 }
 
 void ASTDeclWriter::VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
-  VisitNamedDecl(D);
+  VisitDecl(D);
   Writer.AddSourceLocation(D->getUsingLoc(), Record);
   Writer.AddSourceLocation(D->getNamespaceKeyLocation(), Record);
   Writer.AddNestedNameSpecifierLoc(D->getQualifierLoc(), Record);
   Writer.AddDeclRef(D->getNominatedNamespace(), Record);
   Writer.AddDeclRef(dyn_cast<Decl>(D->getCommonAncestor()), Record);
+  Writer.AddDeclRef(D->getPrev(), Record);
+  if (NamespaceDecl *N = dyn_cast<NamespaceDecl>(D->getDeclContext())) {
+    if (N->LastUsingDirective == D)
+      Writer.AddDeclRef(N, Record);
+    else
+      Writer.AddDeclRef(0, Record);
+  } else {
+    TranslationUnitDecl *TU = cast<TranslationUnitDecl>(D->getDeclContext());
+    if (TU->LastUsingDirective == D)
+      Writer.AddDeclRef(TU, Record);
+    else
+      Writer.AddDeclRef(0, Record);
+  }
+
   Code = serialization::DECL_USING_DIRECTIVE;
 }
 
@@ -1282,7 +1299,6 @@
     // We use the sentinel value 0 to indicate an only declaration.
     Record.push_back(0);
   }
-  
 }
 
 //===----------------------------------------------------------------------===//
@@ -1657,6 +1673,9 @@
   // An ObjCMethodDecl is never considered as "required" because its
   // implementation container always is.
 
+  if (const UsingDirectiveDecl *U = dyn_cast<UsingDirectiveDecl>(D))
+    return isa<TranslationUnitDecl>(U->getDeclContext());
+
   // File scoped assembly or obj-c implementation must be seen.
   if (isa<FileScopeAsmDecl>(D) || isa<ObjCImplDecl>(D))
     return true;
@@ -1680,7 +1699,7 @@
     if (IDR == 0)
       IDR = NextDeclID++;
     
-    ID= IDR;
+    ID = IDR;
   }
 
   bool isReplacingADecl = ID < FirstDeclID;
Index: lib/Serialization/ASTWriter.cpp
===================================================================
--- lib/Serialization/ASTWriter.cpp	(revision 165388)
+++ lib/Serialization/ASTWriter.cpp	(working copy)
@@ -2750,8 +2750,6 @@
       break;
     case DeclarationName::CXXLiteralOperatorName:
       ID.AddString(Name.getCXXLiteralIdentifier()->getName());
-    case DeclarationName::CXXUsingDirective:
-      break;
     }
 
     return ID.ComputeHash();
@@ -2775,7 +2773,6 @@
     case DeclarationName::CXXConstructorName:
     case DeclarationName::CXXDestructorName:
     case DeclarationName::CXXConversionFunctionName:
-    case DeclarationName::CXXUsingDirective:
       break;
     }
     clang::io::Emit16(Out, KeyLen);
@@ -2811,7 +2808,6 @@
     case DeclarationName::CXXConstructorName:
     case DeclarationName::CXXDestructorName:
     case DeclarationName::CXXConversionFunctionName:
-    case DeclarationName::CXXUsingDirective:
       return;
     }
 
@@ -3319,7 +3315,7 @@
   // the results at the end of the chain.
   RecordData WeakUndeclaredIdentifiers;
   if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
-    for (llvm::DenseMap<IdentifierInfo*,WeakInfo>::iterator
+    for (llvm::DenseMap<IdentifierInfo *, WeakInfo>::iterator
          I = SemaRef.WeakUndeclaredIdentifiers.begin(),
          E = SemaRef.WeakUndeclaredIdentifiers.end(); I != E; ++I) {
       AddIdentifierRef(I->first, WeakUndeclaredIdentifiers);
@@ -4016,10 +4012,6 @@
   case DeclarationName::CXXLiteralOperatorName:
     AddIdentifierRef(Name.getCXXLiteralIdentifier(), Record);
     break;
-
-  case DeclarationName::CXXUsingDirective:
-    // No extra data to emit
-    break;
   }
 }
 
@@ -4051,7 +4043,6 @@
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
-  case DeclarationName::CXXUsingDirective:
     break;
   }
 }
Index: lib/Frontend/ChainedIncludesSource.cpp
===================================================================
--- lib/Frontend/ChainedIncludesSource.cpp	(revision 165388)
+++ lib/Frontend/ChainedIncludesSource.cpp	(working copy)
@@ -1,4 +1,4 @@
-//===- ChainedIncludesSource.cpp - Chained PCHs in Memory -------*- C++ -*-===//
+//===- ChainedIncludesSource.cpp - Chained PCHs in Memory -----------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
@@ -86,7 +86,7 @@
                                                                  IK));
 
     TextDiagnosticPrinter *DiagClient =
-      new TextDiagnosticPrinter(llvm::errs(), DiagnosticOptions());
+        new TextDiagnosticPrinter(llvm::errs(), DiagnosticOptions());
     IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
     IntrusiveRefCntPtr<DiagnosticsEngine> Diags(
         new DiagnosticsEngine(DiagID, DiagClient));
@@ -149,7 +149,7 @@
     Clang->getDiagnosticClient().EndSourceFile();
     serialBufs.push_back(
       llvm::MemoryBuffer::getMemBufferCopy(StringRef(serialAST.data(),
-                                                           serialAST.size())));
+                                                     serialAST.size())));
     source->CIs.push_back(Clang.take());
   }
 
Index: lib/Frontend/ChainedDiagnosticConsumer.cpp
===================================================================
--- lib/Frontend/ChainedDiagnosticConsumer.cpp	(revision 165388)
+++ lib/Frontend/ChainedDiagnosticConsumer.cpp	(working copy)
@@ -1,4 +1,4 @@
-//===- ChainedDiagnosticConsumer.cpp - Chain Diagnostic Clients -*- C++ -*-===//
+//===- ChainedDiagnosticConsumer.cpp - Chain Diagnostic Clients -----------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
Index: lib/Frontend/VerifyDiagnosticConsumer.cpp
===================================================================
--- lib/Frontend/VerifyDiagnosticConsumer.cpp	(revision 165388)
+++ lib/Frontend/VerifyDiagnosticConsumer.cpp	(working copy)
@@ -193,8 +193,7 @@
   llvm::Regex Regex;
 };
 
-class ParseHelper
-{
+class ParseHelper {
 public:
   ParseHelper(StringRef S)
     : Begin(S.begin()), End(S.end()), C(Begin), P(Begin), PEnd(NULL) { }
@@ -423,7 +422,7 @@
 }
 
 /// HandleComment - Hook into the preprocessor and extract comments containing
-///  expected errors and warnings.
+/// expected errors and warnings.
 bool VerifyDiagnosticConsumer::HandleComment(Preprocessor &PP,
                                              SourceRange Comment) {
   SourceManager &SM = PP.getSourceManager();
Index: lib/AST/MicrosoftMangle.cpp
===================================================================
--- lib/AST/MicrosoftMangle.cpp	(revision 165388)
+++ lib/AST/MicrosoftMangle.cpp	(working copy)
@@ -521,9 +521,6 @@
       Diags.Report(ND->getLocation(), DiagID);
       break;
     }
-      
-    case DeclarationName::CXXUsingDirective:
-      llvm_unreachable("Can't mangle a using directive name!");
   }
 }
 
Index: lib/AST/ItaniumMangle.cpp
===================================================================
--- lib/AST/ItaniumMangle.cpp	(revision 165388)
+++ lib/AST/ItaniumMangle.cpp	(working copy)
@@ -1188,9 +1188,6 @@
     Out << "li";
     mangleSourceName(Name.getCXXLiteralIdentifier());
     break;
-
-  case DeclarationName::CXXUsingDirective:
-    llvm_unreachable("Can't mangle a using directive name!");
   }
 }
 
Index: lib/AST/DeclarationName.cpp
===================================================================
--- lib/AST/DeclarationName.cpp	(revision 165388)
+++ lib/AST/DeclarationName.cpp	(working copy)
@@ -125,9 +125,6 @@
   case DeclarationName::CXXLiteralOperatorName:
     return LHS.getCXXLiteralIdentifier()->getName().compare(
                                    RHS.getCXXLiteralIdentifier()->getName());
-              
-  case DeclarationName::CXXUsingDirective:
-    return 0;
   }
 
   llvm_unreachable("Invalid DeclarationName Kind!");
@@ -155,13 +152,10 @@
     case DeclarationNameExtra::CXXLiteralOperator:
       return CXXLiteralOperatorName;
 
-    case DeclarationNameExtra::CXXUsingDirective:
-      return CXXUsingDirective;
-
     default:
       // Check if we have one of the CXXOperator* enumeration values.
       if (getExtra()->ExtraKindOrNumArgs <
-            DeclarationNameExtra::CXXUsingDirective)
+            DeclarationNameExtra::NUM_EXTRA_KINDS)
         return CXXOperatorName;
 
       return ObjCMultiArgSelector;
@@ -246,9 +240,6 @@
       OS << Type.getAsString();
     return;
   }
-  case CXXUsingDirective:
-    OS << "<using-directive>";
-    return;
   }
 
   llvm_unreachable("Unexpected declaration name kind");
@@ -324,17 +315,6 @@
   }
 }
 
-DeclarationName DeclarationName::getUsingDirectiveName() {
-  // Single instance of DeclarationNameExtra for using-directive
-  static const DeclarationNameExtra UDirExtra =
-    { DeclarationNameExtra::CXXUsingDirective };
-
-  uintptr_t Ptr = reinterpret_cast<uintptr_t>(&UDirExtra);
-  Ptr |= StoredDeclarationNameExtra;
-
-  return DeclarationName(Ptr);
-}
-
 void DeclarationName::dump() const {
   printName(llvm::errs());
   llvm::errs() << '\n';
@@ -457,8 +437,6 @@
   case DeclarationName::ObjCMultiArgSelector:
     // FIXME: ?
     break;
-  case DeclarationName::CXXUsingDirective:
-    break;
   }
 }
 
@@ -470,7 +448,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     return false;
 
   case DeclarationName::CXXConstructorName:
@@ -492,7 +469,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     return false;
     
   case DeclarationName::CXXConstructorName:
@@ -521,7 +497,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     Name.printName(OS);
     return;
 
@@ -569,7 +544,6 @@
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
-  case DeclarationName::CXXUsingDirective:
     return NameLoc;
   }
   llvm_unreachable("Unexpected declaration name kind");
Index: lib/AST/DeclCXX.cpp
===================================================================
--- lib/AST/DeclCXX.cpp	(revision 165388)
+++ lib/AST/DeclCXX.cpp	(working copy)
@@ -1862,7 +1862,8 @@
                              SourceLocation IdLoc, IdentifierInfo *Id,
                              NamespaceDecl *PrevDecl)
   : NamedDecl(Namespace, DC, IdLoc, Id), DeclContext(Namespace),
-    LocStart(StartLoc), RBraceLoc(), AnonOrFirstNamespaceAndInline(0, Inline) 
+    LocStart(StartLoc), RBraceLoc(), AnonOrFirstNamespaceAndInline(0, Inline),
+    LastUsingDirective(0)
 {
   setPreviousDeclaration(PrevDecl);
   
@@ -1883,6 +1884,11 @@
                                  0, 0);
 }
 
+void NamespaceDecl::addUsingDirective(UsingDirectiveDecl *D) {
+  D->setPrev(LastUsingDirective);
+  LastUsingDirective = D;
+}
+
 void NamespaceAliasDecl::anchor() { }
 
 NamespaceAliasDecl *NamespaceAliasDecl::Create(ASTContext &C, DeclContext *DC,
Index: lib/AST/DeclBase.cpp
===================================================================
--- lib/AST/DeclBase.cpp	(revision 165388)
+++ lib/AST/DeclBase.cpp	(working copy)
@@ -1054,7 +1054,7 @@
   }
 
   // Notify a C++ record declaration that we've added a member, so it can
-  // update it's class-specific state.
+  // update its class-specific state.
   if (CXXRecordDecl *Record = dyn_cast<CXXRecordDecl>(this))
     Record->addedMember(D);
 
@@ -1069,6 +1069,18 @@
 void DeclContext::addDecl(Decl *D) {
   addHiddenDecl(D);
 
+  if (UsingDirectiveDecl *UD = dyn_cast<UsingDirectiveDecl>(D)) {
+    DeclContext *Ctx = UD->getDeclContext();
+    while (!Ctx->isFileContext()) {
+      if (Ctx->isFunctionOrMethod()) {
+        // Nothing to do, let Scope handle it.
+        return;
+      }
+      Ctx = Ctx->getParent();
+    }
+    Ctx->getPrimaryContext()->addUsingDirective(UD);
+  }
+
   if (NamedDecl *ND = dyn_cast<NamedDecl>(D))
     ND->getDeclContext()->getPrimaryContext()->
         makeDeclVisibleInContextWithFlags(ND, false, true);
@@ -1077,6 +1089,18 @@
 void DeclContext::addDeclInternal(Decl *D) {
   addHiddenDecl(D);
 
+  if (UsingDirectiveDecl *UD = dyn_cast<UsingDirectiveDecl>(D)) {
+    DeclContext *Ctx = UD->getDeclContext();
+    while (!Ctx->isFileContext()) {
+      if (Ctx->isFunctionOrMethod()) {
+        // Nothing to do, let Scope handle it.
+        return;
+      }
+      Ctx = Ctx->getParent();
+    }
+    Ctx->getPrimaryContext()->addUsingDirective(UD);
+  }
+
   if (NamedDecl *ND = dyn_cast<NamedDecl>(D))
     ND->getDeclContext()->getPrimaryContext()->
         makeDeclVisibleInContextWithFlags(ND, true, true);
@@ -1091,8 +1115,7 @@
 
   // Skip entities that can't be found by name lookup into a particular
   // context.
-  if ((D->getIdentifierNamespace() == 0 && !isa<UsingDirectiveDecl>(D)) ||
-      D->isTemplateParameter())
+  if (D->getIdentifierNamespace() == 0 || D->isTemplateParameter())
     return true;
 
   // Skip template specializations.
@@ -1354,15 +1377,23 @@
   DeclNameEntries.AddSubsequentDecl(D);
 }
 
+void DeclContext::addUsingDirective(UsingDirectiveDecl *D) {
+  if (TranslationUnitDecl *TU = dyn_cast<TranslationUnitDecl>(this))
+    return TU->addUsingDirective(D);
+  NamespaceDecl *N = cast<NamespaceDecl>(this);
+  return N->addUsingDirective(D);
+}
+
 /// Returns iterator range [First, Last) of UsingDirectiveDecls stored within
 /// this context.
 DeclContext::udir_iterator_range
 DeclContext::getUsingDirectives() const {
-  // FIXME: Use something more efficient than normal lookup for using
-  // directives. In C++, using directives are looked up more than anything else.
-  lookup_const_result Result = lookup(UsingDirectiveDecl::getName());
-  return udir_iterator_range(reinterpret_cast<udir_iterator>(Result.first),
-                             reinterpret_cast<udir_iterator>(Result.second));
+  if (getPrimaryContext() != this)
+    return getPrimaryContext()->getUsingDirectives();
+  if (const TranslationUnitDecl *TU = dyn_cast<TranslationUnitDecl>(this))
+    return TU->getUsingDirectives();
+  const NamespaceDecl *N = cast<NamespaceDecl>(this);
+  return N->getUsingDirectives();
 }
 
 //===----------------------------------------------------------------------===//
Index: lib/AST/ASTImporter.cpp
===================================================================
--- lib/AST/ASTImporter.cpp	(revision 165388)
+++ lib/AST/ASTImporter.cpp	(working copy)
@@ -1768,7 +1768,6 @@
   case DeclarationName::ObjCZeroArgSelector:
   case DeclarationName::ObjCOneArgSelector:
   case DeclarationName::ObjCMultiArgSelector:
-  case DeclarationName::CXXUsingDirective:
     return;
 
   case DeclarationName::CXXOperatorName: {
@@ -4635,10 +4634,6 @@
   case DeclarationName::CXXLiteralOperatorName:
     return ToContext.DeclarationNames.getCXXLiteralOperatorName(
                                    Import(FromName.getCXXLiteralIdentifier()));
-
-  case DeclarationName::CXXUsingDirective:
-    // FIXME: STATICS!
-    return DeclarationName::getUsingDirectiveName();
   }
 
   llvm_unreachable("Invalid DeclarationName Kind!");
Index: lib/AST/Decl.cpp
===================================================================
--- lib/AST/Decl.cpp	(revision 165388)
+++ lib/AST/Decl.cpp	(working copy)
@@ -2859,6 +2859,11 @@
   return new (C) TranslationUnitDecl(C);
 }
 
+void TranslationUnitDecl::addUsingDirective(UsingDirectiveDecl *D) {
+  D->setPrev(LastUsingDirective);
+  LastUsingDirective = D;
+}
+
 void LabelDecl::anchor() { }
 
 LabelDecl *LabelDecl::Create(ASTContext &C, DeclContext *DC,
Index: test/PCH/chain-using-directive.cpp
===================================================================
--- test/PCH/chain-using-directive.cpp	(revision 0)
+++ test/PCH/chain-using-directive.cpp	(working copy)
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -include %s -include %s -fsyntax-only %s -verify
+// RUN: %clang_cc1 -chain-include %s -chain-include %s -fsyntax-only %s -verify
+
+#if !defined(PASS1)
+#define PASS1
+namespace A {
+  int i;
+}
+namespace B {
+  using namespace A;
+  int i;
+}
+namespace C {
+  int i;
+  int j;
+}
+#elif !defined(PASS2)
+#define PASS2
+namespace B {
+  using namespace C;
+  int j;
+}
+#else
+void test()
+{
+  using namespace B;
+  i = 7;  // expected-error {{reference to 'i' is ambiguous}}
+          // expected-note@7{{candidate found}}
+          // expected-note@11{{candidate found}}
+          // expected-note@14{{candidate found}}
+  j = 8;  // expected-error {{reference to 'j' is ambiguous}}
+          // expected-note@15{{candidate found}}
+          // expected-note@21{{candidate found}}
+}
+#endif
Index: include/clang/AST/DeclCXX.h
===================================================================
--- include/clang/AST/DeclCXX.h	(revision 165388)
+++ include/clang/AST/DeclCXX.h	(working copy)
@@ -2375,10 +2375,7 @@
 ///
 ///    using namespace std;
 ///
-// NB: UsingDirectiveDecl should be Decl not NamedDecl, but we provide
-// artificial names for all using-directives in order to store
-// them in DeclContext effectively.
-class UsingDirectiveDecl : public NamedDecl {
+class UsingDirectiveDecl : public Decl {
   virtual void anchor();
   /// \brief The location of the "using" keyword.
   SourceLocation UsingLoc;
@@ -2396,12 +2393,8 @@
   /// namespace.
   DeclContext *CommonAncestor;
 
-  /// getUsingDirectiveName - Returns special DeclarationName used by
-  /// using-directives. This is only used by DeclContext for storing
-  /// UsingDirectiveDecls in its lookup structure.
-  static DeclarationName getName() {
-    return DeclarationName::getUsingDirectiveName();
-  }
+  /// The previous using-directive declaration in the same DeclContext.
+  UsingDirectiveDecl *Prev;
 
   UsingDirectiveDecl(DeclContext *DC, SourceLocation UsingLoc,
                      SourceLocation NamespcLoc,
@@ -2409,9 +2402,10 @@
                      SourceLocation IdentLoc,
                      NamedDecl *Nominated,
                      DeclContext *CommonAncestor)
-    : NamedDecl(UsingDirective, DC, IdentLoc, getName()), UsingLoc(UsingLoc),
+    : Decl(UsingDirective, DC, IdentLoc), UsingLoc(UsingLoc),
       NamespaceLoc(NamespcLoc), QualifierLoc(QualifierLoc),
-      NominatedNamespace(Nominated), CommonAncestor(CommonAncestor) { }
+      NominatedNamespace(Nominated), CommonAncestor(CommonAncestor),
+      Prev(0) { }
 
 public:
   /// \brief Retrieve the nested-name-specifier that qualifies the
@@ -2451,6 +2445,12 @@
   /// getIdentLocation - Returns location of identifier.
   SourceLocation getIdentLocation() const { return getLocation(); }
 
+  void setPrev(UsingDirectiveDecl *UD) {
+    assert(Prev == 0 && "UsingDirectiveDecl may only be a member of one chain");
+    Prev = UD;
+  }
+  UsingDirectiveDecl *getPrev() const { return Prev; }
+
   static UsingDirectiveDecl *Create(ASTContext &C, DeclContext *DC,
                                     SourceLocation UsingLoc,
                                     SourceLocation NamespaceLoc,
@@ -2468,9 +2468,6 @@
   static bool classof(const UsingDirectiveDecl *D) { return true; }
   static bool classofKind(Kind K) { return K == UsingDirective; }
 
-  // Friend for getUsingDirectiveName.
-  friend class DeclContext;
-
   friend class ASTDeclReader;
 };
 
Index: include/clang/AST/DeclBase.h
===================================================================
--- include/clang/AST/DeclBase.h	(revision 165388)
+++ include/clang/AST/DeclBase.h	(working copy)
@@ -1457,10 +1457,46 @@
 
   all_lookups_iterator lookups_end() const;
 
+  /// @brief Makes a using-directive declaration visible within this context.
+  ///
+  /// This routine adds a using directive to this context.
+  void addUsingDirective(UsingDirectiveDecl *D);
+
   /// udir_iterator - Iterates through the using-directives stored
   /// within this context.
-  typedef UsingDirectiveDecl * const * udir_iterator;
+  class udir_iterator {
+    /// Current - The current declaration.
+    UsingDirectiveDecl *Current;
 
+  public:
+    typedef UsingDirectiveDecl*       value_type;
+    typedef UsingDirectiveDecl*       reference;
+    typedef UsingDirectiveDecl*       pointer;
+    typedef std::forward_iterator_tag iterator_category;
+    typedef std::ptrdiff_t            difference_type;
+
+    udir_iterator() : Current(0) { }
+    explicit udir_iterator(UsingDirectiveDecl *D) : Current(D) { }
+
+    reference operator*() const { return Current; }
+    pointer operator->() const { return Current; }
+
+    // Implemented in DeclContextInternals.h
+    udir_iterator& operator++();
+
+    udir_iterator operator++(int) {
+      udir_iterator tmp(*this);
+      ++(*this);
+      return tmp;
+    }
+
+    friend bool operator==(udir_iterator x, udir_iterator y) {
+      return x.Current == y.Current;
+    }
+    friend bool operator!=(udir_iterator x, udir_iterator y) {
+      return x.Current != y.Current;
+    }
+  };
   typedef std::pair<udir_iterator, udir_iterator> udir_iterator_range;
 
   udir_iterator_range getUsingDirectives() const;
Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h	(revision 165388)
+++ include/clang/AST/Decl.h	(working copy)
@@ -75,16 +75,26 @@
   /// translation unit, if one has been created.
   NamespaceDecl *AnonymousNamespace;
 
+  /// The last in a chain of using directives.
+  UsingDirectiveDecl *LastUsingDirective;
+
   explicit TranslationUnitDecl(ASTContext &ctx)
     : Decl(TranslationUnit, 0, SourceLocation()),
       DeclContext(TranslationUnit),
-      Ctx(ctx), AnonymousNamespace(0) {}
+      Ctx(ctx), AnonymousNamespace(0), LastUsingDirective(0) {}
 public:
   ASTContext &getASTContext() const { return Ctx; }
 
   NamespaceDecl *getAnonymousNamespace() const { return AnonymousNamespace; }
   void setAnonymousNamespace(NamespaceDecl *D) { AnonymousNamespace = D; }
 
+  void addUsingDirective(UsingDirectiveDecl *D);
+
+  udir_iterator_range getUsingDirectives() const {
+    return udir_iterator_range(udir_iterator(LastUsingDirective),
+                               udir_iterator());
+  }
+
   static TranslationUnitDecl *Create(ASTContext &C);
   // Implement isa/cast/dyncast/etc.
   static bool classof(const Decl *D) { return classofKind(D->getKind()); }
@@ -96,6 +106,9 @@
   static TranslationUnitDecl *castFromDeclContext(const DeclContext *DC) {
     return static_cast<TranslationUnitDecl *>(const_cast<DeclContext*>(DC));
   }
+
+  friend class ASTDeclReader;
+  friend class ASTDeclWriter;
 };
 
 /// NamedDecl - This represents a decl with a name.  Many decls have names such
@@ -408,6 +421,9 @@
   /// boolean value indicating whether this is an inline namespace.
   llvm::PointerIntPair<NamespaceDecl *, 1, bool> AnonOrFirstNamespaceAndInline;
 
+  /// The last in a chain of using directives.
+  UsingDirectiveDecl *LastUsingDirective;
+
   NamespaceDecl(DeclContext *DC, bool Inline, SourceLocation StartLoc,
                 SourceLocation IdLoc, IdentifierInfo *Id,
                 NamespaceDecl *PrevDecl);
@@ -501,6 +517,13 @@
     return getOriginalNamespace();
   }
   
+  void addUsingDirective(UsingDirectiveDecl *D);
+
+  udir_iterator_range getUsingDirectives() const {
+    return udir_iterator_range(udir_iterator(LastUsingDirective),
+                               udir_iterator());
+  }
+
   virtual SourceRange getSourceRange() const LLVM_READONLY {
     return SourceRange(LocStart, RBraceLoc);
   }
Index: include/clang/AST/DeclContextInternals.h
===================================================================
--- include/clang/AST/DeclContextInternals.h	(revision 165388)
+++ include/clang/AST/DeclContextInternals.h	(working copy)
@@ -159,11 +159,6 @@
 
     DeclsTy &Vec = *getAsVector();
 
-    // Using directives end up in a special entry which contains only
-    // other using directives, so all this logic is wasted for them.
-    // But avoiding the logic wastes time in the far-more-common case
-    // that we're *not* adding a new using directive.
-
     // Tag declarations always go at the end of the list so that an
     // iterator which points at the first tag will start a span of
     // decls that only contains tags.
@@ -218,6 +213,12 @@
   DependentDiagnostic *FirstDiagnostic;
 };
 
+inline DeclContext::udir_iterator& DeclContext::udir_iterator::operator++() {
+  assert(Current && "Advancing while iterator has reached end");
+  Current = Current->getPrev();
+  return *this;
+}
+
 } // end namespace clang
 
 #endif
Index: include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- include/clang/AST/RecursiveASTVisitor.h	(revision 165388)
+++ include/clang/AST/RecursiveASTVisitor.h	(working copy)
@@ -697,7 +697,6 @@
   case DeclarationName::ObjCMultiArgSelector:
   case DeclarationName::CXXOperatorName:
   case DeclarationName::CXXLiteralOperatorName:
-  case DeclarationName::CXXUsingDirective:
     break;
   }
 
Index: include/clang/AST/DeclLookups.h
===================================================================
--- include/clang/AST/DeclLookups.h	(revision 165388)
+++ include/clang/AST/DeclLookups.h	(working copy)
@@ -16,7 +16,6 @@
 
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclContextInternals.h"
-#include "clang/AST/DeclarationName.h"
 
 namespace clang {
 
@@ -40,14 +39,7 @@
   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());
-             
+    ++It;
     return *this;
   }
 
Index: include/clang/AST/DeclarationName.h
===================================================================
--- include/clang/AST/DeclarationName.h	(revision 165388)
+++ include/clang/AST/DeclarationName.h	(working copy)
@@ -30,7 +30,6 @@
   class DeclarationNameExtra;
   class IdentifierInfo;
   class MultiKeywordSelector;
-  class UsingDirectiveDecl;
   class TypeSourceInfo;
 
 /// DeclarationName - The name of a declaration. In the common case,
@@ -51,8 +50,7 @@
     CXXDestructorName,
     CXXConversionFunctionName,
     CXXOperatorName,
-    CXXLiteralOperatorName,
-    CXXUsingDirective
+    CXXLiteralOperatorName
   };
 
 private:
@@ -173,9 +171,6 @@
   // Construct a declaration name from an Objective-C selector.
   DeclarationName(Selector Sel) : Ptr(Sel.InfoPtr) { }
 
-  /// getUsingDirectiveName - Return name for all using-directives.
-  static DeclarationName getUsingDirectiveName();
-
   // operator bool() - Evaluates true when this declaration name is
   // non-empty.
   operator bool() const {
@@ -408,7 +403,6 @@
       unsigned OpNameLoc;
     } CXXLiteralOperatorName;
 
-    // struct {} CXXUsingDirective;
     // struct {} ObjCZeroArgSelector;
     // struct {} ObjCOneArgSelector;
     // struct {} ObjCMultiArgSelector;
Index: include/clang/Basic/DeclNodes.td
===================================================================
--- include/clang/Basic/DeclNodes.td	(revision 165388)
+++ include/clang/Basic/DeclNodes.td	(working copy)
@@ -13,7 +13,6 @@
 def TranslationUnit : Decl, DeclContext;
 def Named : Decl<1>;
   def Namespace : DDecl<Named>, DeclContext;
-  def UsingDirective : DDecl<Named>;
   def NamespaceAlias : DDecl<Named>;
   def Label : DDecl<Named>;
   def Type : DDecl<Named, 1>;
@@ -66,6 +65,7 @@
   def ObjCCompatibleAlias : DDecl<Named>;
 def LinkageSpec : Decl, DeclContext;
 def ObjCPropertyImpl : Decl;
+def UsingDirective : Decl;
 def FileScopeAsm : Decl;
 def AccessSpec : Decl;
 def Friend : Decl;
Index: include/clang/Basic/IdentifierTable.h
===================================================================
--- include/clang/Basic/IdentifierTable.h	(revision 165388)
+++ include/clang/Basic/IdentifierTable.h	(working copy)
@@ -751,7 +751,6 @@
     CXXOperator##Name,
 #include "clang/Basic/OperatorKinds.def"
     CXXLiteralOperator,
-    CXXUsingDirective,
     NUM_EXTRA_KINDS
   };
 
@@ -760,7 +759,6 @@
   /// ExtraKind), in which case the DeclarationNameExtra is also a
   /// CXXSpecialName, (for CXXConstructor, CXXDestructor, or
   /// CXXConversionFunction) CXXOperatorIdName, or CXXLiteralOperatorName,
-  /// it may be also name common to C++ using-directives (CXXUsingDirective),
   /// otherwise it is NUM_EXTRA_KINDS+NumArgs, where NumArgs is the number of
   /// arguments in the Objective-C selector, in which case the
   /// DeclarationNameExtra is also a MultiKeywordSelector.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to