[cfe-commits] r60897 - in /cfe/trunk: include/clang/AST/Decl.h lib/AST/ASTContext.cpp lib/AST/DeclBase.cpp lib/Analysis/RegionStore.cpp lib/CodeGen/CodeGenTypes.cpp lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaInit.cpp lib/Sema/SemaOverload.cpp

[Chris Lattner]

On Dec 11, 2008, at 12:41 PM, Douglas Gregor wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=60897&view=rev
> Log:
> Address some comments on the name lookup/DeclContext patch from Chris

Awesome, thanks Doug!

>
> @@ -227,17 +227,32 @@
>   if (getLangOptions().CPlusPlus && (NS & Decl::IDNS_Ordinary))
>     NS |= Decl::IDNS_Tag;
>
> +  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.

Thanks for doing this.  One question that I thought I'd raise: name  
lookup for labels is sufficiently different (and extremely simple)  
that it might make sense to have a LookupLabelDecl() that is separate  
from LookupDecl().  I think labels can *never* be used with qualified  
names, can never be autosynth'd from builtins, etc.  Also, the call  
sites for LookupDecl all know whether they want a label or not.

Do you think it would make sense to split labels out from other decls  
for lookup?

>
> +    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)

This evaluates .end() each time through the loop :)

> +++ cfe/trunk/lib/Sema/SemaInit.cpp Thu Dec 11 14:41:00 2008
> @@ -41,7 +41,7 @@
>
> 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));

instead of 'const int', how about 'unsigned'?

>

-Chris