On Jun 7, 2013, at 11:43 AM, Adrian Prantl <[email protected]> wrote:

> 
> On Jun 7, 2013, at 11:37 AM, Eric Christopher <[email protected]> wrote:
> 
>> Two quick comments:
>> 
>>> +  assert(Setter->getDeclName().isObjCOneArgSelector());
>>> +  // Construct a setter name like SelectorTable::constructSetterName()
>>> +  // does, but without entering it into the table.
>>> +  SmallString<100> DefaultName("set");
>> 
>> 100 seems a bit big?
>> 
> 
> Well, as the documentation says, this is the exact same code as in 
> SelectorTable::constructSetterName(), and I figured that whoever wrote that 
> put some thought into that constant.
> 
>>> +  DefaultName += PD->getName();
>>> +  DefaultName[3] = toUppercase(DefaultName[3]);
>> 
>> Magic numbers! doing magic things! That we don't mention what they are...
>> 
>> How about some documentation here? :)
> 
> It’s not exactly magic, we are capitalizing the first letter of 
> PD->getName(), the first three characters in DefaultString being “set”.

That being said it might make sense to refactor this into a static method of 
clang::SelectorTable, so we don’t have it implemented in two places.

-- adrian
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to