r183474 - ObjC Debug Info: Emit the names of accessors whenever they diverge from

[Eric Christopher]

On Fri, Jun 7, 2013 at 11:45 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
> On Jun 7, 2013, at 11:43 AM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>> On Jun 7, 2013, at 11:37 AM, Eric Christopher <echristo at gmail.com> 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”.
>

It would be much more obvious if you actually just wrote this in the
code and why it's necessary.

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

Could.

Thanks!

-eric