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

[Richard Smith]

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.
> 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)
>

The way we usually handle this is with both a positive and a negative test:

struct X { int x; } super1;
-(void)objc_method: {
  super.x = 0; // expected-no-error
}
void c_function() {
  super.x = 0; // expected-error {{did you mean 'super1'}}
}


> 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 convinced that this has sufficient value to justify adding a -cc1
option for it. Can you elaborate on why this is important?

I'm also surprised we hit the typo-correction codepath at all in this case;
we shouldn't be getting there unless we've already decided to emit a
diagnostic.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130316/cdad8d01/attachment.html>