[PATCH][ObjC] Cleanup ObjCInterfaceDecl lookup for ObjC literals

John McCall rjmccall at apple.com
Thu Jul 23 13:46:27 PDT 2015


> On Jul 23, 2015, at 1:43 PM, AlexDenisov <1101.debian at gmail.com> wrote:
> 
>> I think it would make more sense to have this take an ObjCLiteralKind
>> and then map that to a NSAPI::NSClassIdKindKind than the other way
>> around.
> 
> It was my initial idea, but I’m stuck with default value in case
> when ObjCLiteralKind doesn’t match any NSClassIdKindKind (e.g.: LK_Block or LK_None).

Feel free to just assert that these aren’t passed by putting an
llvm_unreachable in those cases.

John.

>> You should declare variables as close as possible to the point they’re initialized,
>> and you should try to avoid naming variables the same as a type.
> 
> Will do.
> 
>> On 23 Jul 2015, at 22:29, John McCall <rjmccall at apple.com> wrote:
>> 
>>> On Jul 23, 2015, at 8:33 AM, AlexDenisov <1101.debian at gmail.com> wrote:
>>>> Then, on the Sema side, you should define an enum that corresponds to
>>>> the different cases in that second parameter.
>>> 
>>> I just reused ObjCLiteralKind, seems it fits for this task very well.
>> 
>> Sounds good.
>> 
>>>> If you do this, you should be able to unify all the different diagnostics for
>>>> NSNumber/NSArray/NSValue.
>>> 
>>> Merged all specific errors into one err_undeclared_objc_literal_class, as you suggested.
>>> 
>>> New version attached.
>>> 
>>> <lookup_objc_literal_interface_decl.patch>
>> 
>> +static bool ValidateObjCLiteralInterfaceDecl(Sema &S, ObjCInterfaceDecl *Decl,
>> +                                             SourceLocation Loc,
>> +                                             NSAPI::NSClassIdKindKind ClassKind) {
>> 
>> I think it would make more sense to have this take an ObjCLiteralKind
>> and then map that to a NSAPI::NSClassIdKindKind than the other way
>> around.
>> 
>> +  ObjCInterfaceDecl *Decl = nullptr;
>> +  IdentifierInfo *II = S.NSAPIObj->getNSClassId(ClassKind);
>> +  NamedDecl *IF = S.LookupSingleName(S.TUScope, II, Loc,
>> +                                     Sema::LookupOrdinaryName);
>> +  Decl = dyn_cast_or_null<ObjCInterfaceDecl>(IF);
>> 
>> You should declare variables as close as possible to the point they’re initialized,
>> and you should try to avoid naming variables the same as a type.
>> 
>> So, specifically, please make this
>> ObjCInterfaceDecl *ID = dyn_cast_or_null<ObjCInterfaceDecl>(IF);
>> 
>> Otherwise, this looks good, thanks!
>> 
>> John.
> 





More information about the cfe-commits mailing list