[cfe-commits] [Patch] Suggest typo corrections for implicit function decls

[Richard Smith]

On Mon, December 5, 2011 15:41, Hans Wennborg wrote:
> Thanks very much for the review! Attaching a revised patch; please
> take another look.

LGTM. Thanks!

- Richard

> On Fri, Dec 2, 2011 at 11:09 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp +  // See if we
>> can find a typo correction. +  TypoCorrection Corrected;
>> +  FunctionDecl* Func = NULL;
>>
>>
>> Please put the space to the right of the *, not to the left. Also, the null
>>  pointer constant is written as 0 more frequently than as NULL in Sema.
>
> Fixed.
>
>
>> +    Diag(Loc, diagnostic_kind) << &II << CorrectedQuotedStr
>> +      << FixItHint::CreateReplacement(Loc, CorrectedStr);
>>
>>
>> Please move this fix-it to a separate note (at least for the implicit
>> function declaration case -- see below). When a fix-it is issued on a
>> warning or an error, we must recover as if the fixed code had been parsed
>> (and for a
>> warning, this essentially means the fix-it must not change the semantics).
>
> Done.
>
>
>> [...], it strikes me that a Warning, DefaultError message with no -W flag,
>> like this one, is a strange construct indeed, and it should probably simply
>> be changed into an Error. With that done, you can keep the fix-it on the
>> error for the __builtin_ case, if you also recover as if the user had typed
>> the correct builtin name.
>
> Turned it into an Error.
>
>
> I wasn't sure how to recover to the suggested builtin name from this
> function, so not doing that for now.
>
>> diff --git a/test/Sema/builtins.c b/test/Sema/builtins.c Rather than turning
>> off spell-checking, it'd be better to update the test to expect the new
>> output (and add more cases here if we've lost some coverage).
>
> Done.
>
>
>> Could you add a test for the C89 case, please?
>>
>
> Done.
>
>