Address some comments on the name lookup/DeclContext patch from Chris

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

include/clang/AST/Decl.h

@@ -1106,6 +1106,10 @@ public:
     return field_const_iterator(decls_end(), decls_end());
   }
 
+  // field_empty - Whether there are any fields (non-static data
+  // members) in this record.
+  bool field_empty() const { return field_begin() == field_end(); }
+
   /// completeDefinition - Notes that the definition of this type is
   /// now complete.
   void completeDefinition(ASTContext& C);

lib/AST/ASTContext.cpp

@@ -539,6 +539,7 @@ const ASTRecordLayout &ASTContext::getASTRecordLayout(const RecordDecl *D) {
   ASTRecordLayout *NewEntry = new ASTRecordLayout();
   Entry = NewEntry;
 
+  // FIXME: Avoid linear walk through the fields, if possible.
   NewEntry->InitializeLayout(std::distance(D->field_begin(), D->field_end()));
   bool IsUnion = D->isUnion();
 

lib/AST/DeclBase.cpp

@@ -516,7 +516,7 @@ DeclContext::lookup(ASTContext &Context, DeclarationName Name) {
   // We have a small array. Look into it.
   unsigned Size = LookupPtr.getInt();
   NamedDecl **Array = static_cast<NamedDecl**>(LookupPtr.getPointer());
-  for (unsigned Idx = 0; Idx < Size; ++Idx)
+  for (unsigned Idx = 0; Idx != Size; ++Idx)
     if (Array[Idx]->getDeclName() == Name) {
       Result.first = &Array[Idx];
       Result.second = Result.first + 1;
@@ -566,12 +566,11 @@ void DeclContext::insertImpl(NamedDecl *D) {
     // from lookup(). There will be zero, one, or two declarations of
     // the same name.
     unsigned Match;
-    for (Match = 0; Match < Size; ++Match) {
+    for (Match = 0; Match != Size; ++Match)
       if (Array[Match]->getDeclName() == D->getDeclName())
-       break;
-    }
+        break;
 
-    if (Match < Size) {
+    if (Match != Size) {
       // We found another declaration with the same name. If it's also
       // in the same identifier namespace, update the declaration in
       // place.
@@ -590,7 +589,7 @@ void DeclContext::insertImpl(NamedDecl *D) {
       // declaration for C++ name lookup to operate properly. Therefore,
       // if our match is an ordinary name and the new name is in the
       // tag namespace, we'll insert the new declaration after it. 
-      if (Match < Size && (NS == Decl::IDNS_Tag) && 
+      if (Match != Size && (NS == Decl::IDNS_Tag) && 
          (Array[Match]->getIdentifierNamespace() & Decl::IDNS_Ordinary))
        ++Match;
     }
@@ -611,7 +610,7 @@ void DeclContext::insertImpl(NamedDecl *D) {
     StoredDeclsMap *Map = new StoredDeclsMap(16);
     LookupPtr.setPointer(Map);
     LookupPtr.setInt(LookupIsMap);
-    for (unsigned Idx = 0; Idx < LookupIsMap - 1; ++Idx) 
+    for (unsigned Idx = 0; Idx != LookupIsMap - 1; ++Idx) 
       insertImpl(Array[Idx]);
     delete [] Array;
 
@@ -626,10 +625,9 @@ void DeclContext::insertImpl(NamedDecl *D) {
   if (Pos == Map->end()) {
     // Put this declaration into the appropriate slot.
     TwoNamedDecls Val;
-    Val.Decls[0] = 0;
-    Val.Decls[1] = 0;
     Val.Decls[IndexOfD] = D;
-    Pos = Map->insert(std::make_pair(D->getDeclName(),Val)).first;
+    Val.Decls[!IndexOfD] = 0;
+    Map->insert(std::make_pair(D->getDeclName(),Val)).first;
   } else {
     Pos->second.Decls[IndexOfD] = D;
   }

lib/Analysis/RegionStore.cpp

@@ -433,13 +433,12 @@ SVal RegionStoreManager::RetrieveStruct(Store store, const TypedRegion* R) {
 
   llvm::ImmutableList<SVal> StructVal = getBasicVals().getEmptySValList();
 
-  for (DeclContext::reverse_decl_iterator Mem = RD->decls_rbegin();
-       Mem != RD->decls_rend(); ++Mem) {
-    FieldDecl *FD = dyn_cast<FieldDecl>(*Mem);
-    if (!FD)
-      continue;
+  std::vector<FieldDecl *> Fields(RD->field_begin(), RD->field_end());
 
-    FieldRegion* FR = MRMgr.getFieldRegion(FD, R);
+  for (std::vector<FieldDecl *>::reverse_iterator Field = Fields.rbegin(),
+                                               FieldEnd = Fields.rend();
+       Field != FieldEnd; ++Field) {
+    FieldRegion* FR = MRMgr.getFieldRegion(*Field, R);
     RegionBindingsTy B(static_cast<const RegionBindingsTy::TreeTy*>(store));
     RegionBindingsTy::data_type* data = B.lookup(FR);
 

lib/CodeGen/CodeGenTypes.cpp

@@ -392,7 +392,7 @@ const llvm::Type *CodeGenTypes::ConvertTagDeclType(const TagDecl *TD) {
   } else if (TD->isUnion()) {
     // Just use the largest element of the union, breaking ties with the
     // highest aligned member.
-    if (RD->field_begin() != RD->field_end()) {
+    if (!RD->field_empty()) {
       RecordOrganizer RO(*this, *RD);
       
       RO.layoutUnionFields(Context.getASTRecordLayout(RD));

lib/Sema/SemaCXXScopeSpec.cpp

@@ -15,21 +15,22 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/Parse/DeclSpec.h"
 #include "clang/Basic/Diagnostic.h"
+#include "llvm/ADT/STLExtras.h"
 using namespace clang;
 
 
 namespace {
   Decl *LookupNestedName(DeclContext *LookupCtx, bool LookInParentCtx,
                          DeclarationName Name, bool &IdIsUndeclared,
-                        ASTContext &Context) {
+                         ASTContext &Context) {
     if (LookupCtx && !LookInParentCtx) {
       IdIsUndeclared = true;
-      for (DeclContext::lookup_const_result I = LookupCtx->lookup(Context, Name);
-          I.first != I.second; ++I.first) {
+      DeclContext::lookup_const_iterator I, E;
+      for (llvm::tie(I, E) = LookupCtx->lookup(Context, Name); I != E; ++I) {
        IdIsUndeclared = false;
-       if (((*I.first)->getIdentifierNamespace() & Decl::IDNS_Tag) &&
-           !isa<EnumDecl>(*I.first))
-         return *I.first;
+       if (((*I)->getIdentifierNamespace() & Decl::IDNS_Tag) && 
+           !isa<EnumDecl>(*I))
+         return *I;
       }
 
       return 0;

lib/Sema/SemaDecl.cpp

@@ -25,6 +25,8 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/HeaderSearch.h" 
 #include "llvm/ADT/SmallSet.h"
+#include "llvm/ADT/STLExtras.h"
+
 using namespace clang;
 
 Sema::TypeTy *Sema::isTypeName(IdentifierInfo &II, Scope *S,
@@ -134,14 +136,14 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S) {
         Ovl = OverloadedFunctionDecl::Create(Context, 
                                              FD->getDeclContext(),
                                              FD->getDeclName());
-        Ovl->addOverload(dyn_cast<FunctionDecl>(Prev));
+        Ovl->addOverload(cast<FunctionDecl>(Prev));
         
-       // If there is an name binding for the existing FunctionDecl,
+       // If there is an ame binding for the existing FunctionDecl,
        // remove it.
        for (IdentifierResolver::iterator I 
               = IdResolver.begin(FD->getDeclName(), FD->getDeclContext(), 
-                                 false/*LookInParentCtx*/);
-            I != IdResolver.end(); ++I) {
+                                 false/*LookInParentCtx*/),
+              E = IdResolver.end(); I != E; ++I) {
          if (*I == Prev) {
            IdResolver.RemoveDecl(*I);
            S->RemoveDecl(*I);
@@ -175,13 +177,11 @@ void Sema::PushOnScopeChains(NamedDecl *D, Scope *S) {
     }
   }
 
+  // Add scoped declarations into their context, so that they can be
+  // found later. Declarations without a context won't be inserted
+  // into any context.
   if (ScopedDecl *SD = dyn_cast<ScopedDecl>(D))
     CurContext->addDecl(Context, SD);
-  else {
-    // Other kinds of declarations don't currently have a context
-    // where they need to be inserted.
-  }
-
 
   IdResolver.AddDecl(D);
 }
@@ -227,17 +227,32 @@ Decl *Sema::LookupDecl(DeclarationName Name, unsigned NSI, Scope *S,
   if (getLangOptions().CPlusPlus && (NS & Decl::IDNS_Ordinary))
     NS |= Decl::IDNS_Tag;
 
-  if (LookupCtx) {
+  if (LookupCtx == 0 && 
+      (!getLangOptions().CPlusPlus || (NS == Decl::IDNS_Label))) {
+    // Unqualified name lookup in C/Objective-C and name lookup for
+    // labels in C++ is purely lexical, so search in the
+    // declarations attached to the name.
+    assert(!LookupCtx && "Can't perform qualified name lookup here");
+    IdentifierResolver::iterator I
+      = IdResolver.begin(Name, CurContext, LookInParent);
+    
+    // Scan up the scope chain looking for a decl that matches this
+    // identifier that is in the appropriate namespace.  This search
+    // should not take long, as shadowing of names is uncommon, and
+    // deep shadowing is extremely uncommon.
+    for (; I != IdResolver.end(); ++I)
+      if ((*I)->getIdentifierNamespace() & NS)
+         return *I;
+  } else if (LookupCtx) {
     assert(getLangOptions().CPlusPlus && "No qualified name lookup in C");
 
     // Perform qualified name lookup into the LookupCtx.
     // FIXME: Will need to look into base classes and such.
-    for (DeclContext::lookup_const_result I = LookupCtx->lookup(Context, Name);
-        I.first != I.second; ++I.first)
-      if ((*I.first)->getIdentifierNamespace() & NS)
-       return *I.first;
-  } else if (getLangOptions().CPlusPlus && 
-               (NS & (Decl::IDNS_Ordinary | Decl::IDNS_Tag))) {
+    DeclContext::lookup_const_iterator I, E;
+    for (llvm::tie(I, E) = LookupCtx->lookup(Context, Name); I != E; ++I)
+      if ((*I)->getIdentifierNamespace() & NS)
+       return *I;
+  } else {
     // Name lookup for ordinary names and tag names in C++ requires
     // looking into scopes that aren't strictly lexical, and
     // therefore we walk through the context as well as walking
@@ -260,11 +275,11 @@ Decl *Sema::LookupDecl(DeclarationName Name, unsigned NSI, Scope *S,
         Ctx = Ctx->getParent();
       while (Ctx && (Ctx->isNamespace() || Ctx->isCXXRecord())) {
         // Look for declarations of this name in this scope.
-        for (DeclContext::lookup_const_result I = Ctx->lookup(Context, Name);
-             I.first != I.second; ++I.first) {
+        DeclContext::lookup_const_iterator I, E;
+        for (llvm::tie(I, E) = Ctx->lookup(Context, Name); I != E; ++I) {
           // FIXME: Cache this result in the IdResolver
-          if ((*I.first)->getIdentifierNamespace() & NS)
-            return  *I.first;
+          if ((*I)->getIdentifierNamespace() & NS)
+            return  *I;
         }
         
         Ctx = Ctx->getParent();
@@ -273,22 +288,6 @@ Decl *Sema::LookupDecl(DeclarationName Name, unsigned NSI, Scope *S,
       if (!LookInParent)
         return 0;
     }
-  } else {
-    // Unqualified name lookup for names in our lexical scope. This
-    // name lookup suffices when all of the potential names are known
-    // to have been pushed onto the IdResolver, as happens in C
-    // (always) and in C++ for names in the "label" namespace.
-    assert(!LookupCtx && "Can't perform qualified name lookup here");
-    IdentifierResolver::iterator I
-      = IdResolver.begin(Name, CurContext, LookInParent);
-    
-    // Scan up the scope chain looking for a decl that matches this
-    // identifier that is in the appropriate namespace.  This search
-    // should not take long, as shadowing of names is uncommon, and
-    // deep shadowing is extremely uncommon.
-    for (; I != IdResolver.end(); ++I)
-      if ((*I)->getIdentifierNamespace() & NS)
-         return *I;
   }
 
   // If we didn't find a use of this identifier, and if the identifier

lib/Sema/SemaDeclCXX.cpp

@@ -1268,8 +1268,8 @@ Sema::DeclTy *Sema::ActOnStartNamespaceDef(Scope *NamespcScope,
 
       // Remove the previous declaration from the scope.      
       if (DeclRegionScope->isDeclScope(OrigNS)) {
-       IdResolver.RemoveDecl(OrigNS);
-       DeclRegionScope->RemoveDecl(OrigNS);
+        IdResolver.RemoveDecl(OrigNS);
+        DeclRegionScope->RemoveDecl(OrigNS);
       }
     } else if (PrevDecl) {
       // This is an invalid name redefinition.

lib/Sema/SemaInit.cpp

@@ -41,7 +41,7 @@ int InitListChecker::numArrayElements(QualType DeclType) {
 
 int InitListChecker::numStructUnionElements(QualType DeclType) {
   RecordDecl *structDecl = DeclType->getAsRecordType()->getDecl();
-  int InitializableMembers 
+  const int InitializableMembers 
     = std::count_if(structDecl->field_begin(), structDecl->field_end(),
                     std::mem_fun(&FieldDecl::getDeclName));
   if (structDecl->isUnion())

lib/Sema/SemaOverload.cpp

@@ -1956,7 +1956,7 @@ void Sema::AddOperatorCandidates(OverloadedOperatorKind Op, Scope *S,
   //        empty.
   if (const RecordType *T1Rec = T1->getAsRecordType()) {
     DeclContext::lookup_const_result Lookup 
-      = cast<CXXRecordType>(T1Rec)->getDecl()->lookup(Context, OpName);
+      = T1Rec->getDecl()->lookup(Context, OpName);
     NamedDecl *MemberOps = (Lookup.first == Lookup.second)? 0 : *Lookup.first;
     if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(MemberOps))
       AddMethodCandidate(Method, Args[0], Args+1, NumArgs - 1, CandidateSet,
@@ -3119,7 +3119,7 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Object,
   OverloadCandidateSet CandidateSet;
   DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(OO_Call);
   DeclContext::lookup_const_result Lookup 
-    = cast<CXXRecordType>(Record)->getDecl()->lookup(Context, OpName);
+    = Record->getDecl()->lookup(Context, OpName);
   NamedDecl *MemberOps = (Lookup.first == Lookup.second)? 0 : *Lookup.first;
   if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(MemberOps))
     AddMethodCandidate(Method, Object, Args, NumArgs, CandidateSet,
@@ -3314,7 +3314,7 @@ Sema::BuildOverloadedArrowExpr(Expr *Base, SourceLocation OpLoc,
   OverloadCandidateSet CandidateSet;
   const RecordType *BaseRecord = Base->getType()->getAsRecordType();
   DeclContext::lookup_const_result Lookup 
-    = cast<CXXRecordType>(BaseRecord)->getDecl()->lookup(Context, OpName);
+    = BaseRecord->getDecl()->lookup(Context, OpName);
   NamedDecl *MemberOps = (Lookup.first == Lookup.second)? 0 : *Lookup.first;
   if (CXXMethodDecl *Method = dyn_cast_or_null<CXXMethodDecl>(MemberOps))
     AddMethodCandidate(Method, Base, 0, 0, CandidateSet,