r177218 - Remove -Wspellcheck and replace it with a diagnostic option.

[David Blaikie]

On Sat, Mar 16, 2013 at 10:32 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> On Mar 16, 2013, at 10:18 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> On Sat, Mar 16, 2013 at 10:13 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>> On Sat, Mar 16, 2013 at 3:40 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>>>> Author: akirtzidis
>>>> Date: Fri Mar 15 20:40:35 2013
>>>> New Revision: 177218
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=177218&view=rev
>>>> Log:
>>>> Remove -Wspellcheck and replace it with a diagnostic option.
>>>>
>>>> Thanks to Richard S. for pointing out that the warning would show up
>>>> with -Weverything.
>>>
>>> If we are going to start testing clang this way, it would be better to
>>> design this first, so that adding new 'testing' diagnostics is easy
>>> *and* does not slow down the normal compilation.  I think the second
>>> part is addressed already.
>>>
>>> For example, adding a command line option every time is excessive.
>>> This option could be renamed to -fclang-debugging-diagnostics, and all
>>> such diagnostics could be placed under a special flag
>>> -Wclang-debugging.
>>
>> I still don't understand the need for this at all. At a glance it
>> seems like we're adding a positive diagnostic so we can check for the
>> absence of a diagnostic - but we've never had a need to do this in the
>> past. "-verify" fails if a diagnostic is emitted where it isn't
>> expected so the absence of expected-blah lines is sufficient to test
>> that we don't emit a diagnostic.
>>
>> Am I missing something here? Why are we doing this?
>
> This code snippet of an objc method
>
> -(void)objc_method: {
>   super.x = 0;
> }
>
> would trigger typo-correction for 'super' silently, without emitting any diagnostic.

If it didn't emit any diagnostics then what was the bug you were fixing?

> For the regression test I added I put:
>
> typedef int super1;
>
> so typo-correction "succeeds" in correcting 'super' to 'super1' and errors are emitted.
> For regression testing purposes this would be sufficient though I don't like that we would be inferring that a typo-correction did not trigger indirectly (it is possible, though unlikely, that typo-correction would trigger without resolving to the intended identifier)

It's not a random chance/likely-ness issue, though. The typo
correction algorithm is known, so I would assume we would test it like
any other.

I realize, of course, that the typo correction algorithm could get
smarter & thus negative cases like this might become negative for
other reasons (eg: because we did more precise typo correction &
didn't suggest an identifier you chose that was similar but "not
similar enough") & then we could regress the original reason & perhaps
eventually trigger typo correction for some other pair of strings in
the same context. That seems a somewhat narrow case, though. (& you
could, if really necessary, provide some guarantees against that by
testing a positive case of the same string pair correction in a
different context where you expect it to fire)

> Beyond regression testing I'd like to have a way to get notified when typo-correction is silently triggered for general testing.

I'm not sure why this feature would be particularly necessary for
typo-correction as opposed to any other feature of the
compiler/diagnostic system.