On Mar 16, 2013, at 3:54 PM, Richard Smith <[email protected]> wrote:
> On Sat, Mar 16, 2013 at 10:32 AM, Argyrios Kyrtzidis <[email protected]> > wrote: > On Mar 16, 2013, at 10:18 AM, David Blaikie <[email protected]> wrote: > > > On Sat, Mar 16, 2013 at 10:13 AM, Dmitri Gribenko <[email protected]> > > wrote: > >> On Sat, Mar 16, 2013 at 3:40 AM, Argyrios Kyrtzidis <[email protected]> > >> 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 not sure what else I could say beyond repeating myself; typo-correction is expensive, triggering it needlessly is unacceptable, a -cc1 option allows making sure that it is not triggered across full project sources. I don't see much complexity or maintenance involved for justifying not having it. > > 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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
