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

[Hans Wennborg]

Thanks very much for the review! Attaching a revised patch; please
take another look.

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implicit-decl-typo-correction2.diff
Type: text/x-patch
Size: 6580 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111205/321a8839/attachment.bin>