On Wed, Feb 8, 2012 at 12:24 PM, Ted Kremenek <[email protected]> wrote: > Out of curiosity, how did this trigger for you? Would you object to this > being under -Wall?
It fired all over the place in the chromium codebase and its dependencies. I just turned it off, so I can live with it being in -Wall. But one thing I really like about clang is its sense for good defaults in warnings. This warning feels like it's fairly noisy on existing code, and fixing it would be done mostly to appease the warning – in my experience, enum-switching code usually isn't terribly buggy. (I've been meaning to look at the recent switch changes and warnings and send a mail about them, but I didn't get to that yet. I feel like moving this warning out of -Wall and moving -Wswitch from -Wmost to -Wall is the right thing to do, but I haven't looked to closely yet.) Maybe that's just change aversion, though :-) Nico > > On Feb 7, 2012, at 9:18 PM, Nico Weber wrote: > >> +1, thanks for this! >> >> On Tue, Feb 7, 2012 at 9:08 PM, Ted Kremenek <[email protected]> wrote: >>> Author: kremenek >>> Date: Tue Feb 7 23:08:58 2012 >>> New Revision: 150055 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=150055&view=rev >>> Log: >>> Move -Wcovered-switch-default out of -Wswitch (and -Wall), and make it an >>> opt-in warning. >>> >>> This is a great warning, but it was observed that a ton of real world code >>> violates >>> it all the time for (semi-)legitimate reasons. This warnings is fairly >>> pedantic, which is good, >>> but not for everyone. For example, there is a fair amount of idiomatic >>> code out there >>> that does "default: abort()", and similar idioms. >>> >>> Addresses <rdar://problem/10814651>. >>> >>> Modified: >>> cfe/trunk/include/clang/Basic/DiagnosticGroups.td >>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> cfe/trunk/test/Sema/switch.c >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=150055&r1=150054&r2=150055&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Feb 7 23:08:58 >>> 2012 >>> @@ -184,9 +184,9 @@ >>> def : DiagGroup<"strict-prototypes">; >>> def StrictSelector : DiagGroup<"strict-selector-match">; >>> def MethodDuplicate : DiagGroup<"duplicate-method-match">; >>> -def SwitchEnum : DiagGroup<"switch-enum">; >>> def CoveredSwitchDefault : DiagGroup<"covered-switch-default">; >>> -def Switch : DiagGroup<"switch", [CoveredSwitchDefault]>; >>> +def SwitchEnum : DiagGroup<"switch-enum">; >>> +def Switch : DiagGroup<"switch">; >>> def Trigraphs : DiagGroup<"trigraphs">; >>> >>> def : DiagGroup<"type-limits">; >>> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=150055&r1=150054&r2=150055&view=diff >>> ============================================================================== >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Feb 7 >>> 23:08:58 2012 >>> @@ -5064,7 +5064,7 @@ >>> >>> def warn_unreachable_default : Warning< >>> "default label in switch which covers all enumeration values">, >>> - InGroup<CoveredSwitchDefault>; >>> + InGroup<CoveredSwitchDefault>, DefaultIgnore; >>> def warn_not_in_enum : Warning<"case value not in enumerated type %0">, >>> InGroup<Switch>; >>> def err_typecheck_statement_requires_scalar : Error< >>> >>> Modified: cfe/trunk/test/Sema/switch.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=150055&r1=150054&r2=150055&view=diff >>> ============================================================================== >>> --- cfe/trunk/test/Sema/switch.c (original) >>> +++ cfe/trunk/test/Sema/switch.c Tue Feb 7 23:08:58 2012 >>> @@ -1,4 +1,4 @@ >>> -// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-enum %s >>> +// RUN: %clang_cc1 -fsyntax-only -verify -Wswitch-enum >>> -Wcovered-switch-default %s >>> void f (int z) { >>> while (z) { >>> default: z--; // expected-error {{statement not in switch}} >>> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
