[cfe-commits] [patch] fixit for null-conversion

[David Blaikie]

On Mon, Apr 30, 2012 at 10:44 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Apr 30, 2012, at 10:39 AM, David Blaikie wrote:
>
>> On Mon, Apr 30, 2012 at 9:05 AM, Douglas Gregor <dgregor at apple.com> wrote:
>>>
>>> On Apr 29, 2012, at 9:05 PM, David Blaikie wrote:
>>>
>>>> Just something I've had lying around for a while - I'm not sure
>>>> whether this meets the fixit bar, presumably the real mistakes people
>>>> make when using NULL is that they were calling the wrong function,
>>>> passing arguments in the wrong order, etc - if they're simply passing
>>>> NULL when they meant 0, that's not really the cause of bugs.
>>>
>>> +const char *Sema::getFixItZeroLiteralForType(QualType T) const {
>>> +  const char *Str = getFixItZeroInitializerForType(T);
>>> +  if (!Str)
>>> +    return Str;
>>> +  size_t Len = std::strlen(Str);
>>> +  assert(Len > 3 && "Should be a non-record type init");
>>> +  assert(strncmp(Str, " = ", 3) == 0 && "Should be non-record type init");
>>> +  return Str + 3;
>>> +}
>>>
>>> This feels very brittle.
>>
>> Fair point. (repeating from IRC for posterity - I think I'd wanted to
>> avoid string concatenation, but since these will all be small that's
>> probably not worth avoiding. Also, the need to return a "non value"
>> (null in this case) meant string was a bit awkward - but the use of
>> the empty string for the non-value has actually tidied some callers up
>> a bit)
>>
>> I've refactored it out into a common function - though it means
>> getFixItZeroLiteralForType is now a trivial wrapper. Should that just
>> be folded into getFixItZeroLiteralForType, or do you prefer the two to
>> use a common function to make the sharing a bit clearer?
>
>
> I'm happy with how it is now.

Works for me. Committed as r155839 - thanks for the quick review.

- David