r183582 - address some comments on r183474:

[Adrian Prantl]

On Jun 7, 2013, at 3:41 PM, Eric Christopher <echristo at gmail.com> wrote:

> Hi Adrian,
> 
> Few more comments:
> 
>> +  static SmallString<100> constructSetterName(StringRef Name);
>> +
> 
> Probably just want to return the string here, Small* are more useful
> when they're stack variables and now you're passing much larger things
> around (given you've set a size of 100 - which I'm reasonably positive
> was just a WAG).

When you say “just return the string” what’s the preferred data type for that, a std::string?

I’m curious what would be a buffer size that you would be happy with? It needs to hold the name of an Objective-C property + 3.

> 
>> +/*static*/ SmallString<100>

> No commented out static please.

Just as with the 100 constant, this was in the original implementation, but I’ve got no problem with removing it :-)

>> +SelectorTable::constructSetterName(StringRef Name) {
>> +  SmallString<100> SelectorName("set");
>> +  SelectorName += Name;
>>   SelectorName[3] = toUppercase(SelectorName[3]);
>> -  IdentifierInfo *SetterName = &Idents.get(SelectorName);
>> +  return SelectorName;
>> 
> 
> Why are we capitalizing SelectorName[3] again?
> 
The doxygen comment for the function is:
  /// \brief Return the default setter name for the given identifier.
  ///
  /// This is "set" + \p Name where the initial character of \p Name
  /// has been capitalized.

and our style guide says
"Don’t duplicate the documentation comment in the header file and in the implementation file.”.

thanks,
adrian