[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

[Douglas Gregor]

On Dec 11, 2008, at 12:56 PM, Chris Lattner wrote:

> 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?

Hah! It turns out that we never actually use IDNS_Label in Clang :).  
Labels go into the LabelMap (indexed by identifier), and never hit the  
IdResolver.

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

You're relentless! :)

>
>> +++ 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'?


Sure.

	- Doug