Should this have a -W flag? Should it be on or off by default? If we're not sure about how valuable this warning is, then we need a flag to turn it off. Then, there is a maintenance cost associated with adding more -W flags.
The kinds of time wasters I used to hit are probably covered by -Wconversion and -Wsign-compare, but I never turned them on. If people agree that there is value to having a mode that tries (best effort, not guaranteed) to warn when MSVC would, what about adding a flag that groups these together? Like -Wmsvc-[123]. Then if we implemented a cl.exe compatible driver, we'd just alias /W3 to it. And people could use clang without breaking 'cl /W[123] /Wx' builds. Then this switch warning and others like it could go under -Wmsvc-3, without leading to a proliferation of -Wreally-obscure-warning flags. I don't think you'd want to take this as far as maintaining a mapping of numbered MSVC warnings to clang warnings. If someone is using 'cl.exe /W3 /wd...' they can come up with an equivalent expression in clang flags. On Mon, Jun 3, 2013 at 11:23 AM, Aaron Ballman <[email protected]>wrote: > Ping? > > I've given this some more thought, and I feel like Reid and Daniel (in > the PR) hit the nail on the head: it is really nice to be able to > write code in clang and not have to worry that I've introduced a > warning in MSVC, especially since I try to compile everything with > warnings as errors. I understand the fear of trying to match > diagnostics with other compilers, but where's the harm if it's not > overly chatty and it can catch bugs? > > Also, I think the warning adds value over the empty statement warning > because the situations are different. Eg) > > switch (expr); // Should warn because the ; is suspicious > { > /* stuff */ > } > > // vs > > switch (expr) { > /* ToDo: finish this */ > } // Should warn because the emptiness of the block is suspicious > > > In the case where the user really does want an empty switch statement, > they can selectively turn the diagnostic off. This really shouldn't > be a common case, after all. > > ~Aaron > > On Wed, May 29, 2013 at 4:10 PM, Reid Kleckner <[email protected]> wrote: > > On Wed, May 29, 2013 at 3:49 PM, Richard Smith <[email protected]> > > wrote: > >> > >> On Wed, May 29, 2013 at 7:35 AM, Aaron Ballman <[email protected]> > >> wrote: > >>> > >>> -Wunreachable-code doesn't warn on empty statement bodies for the > >>> switch. So, for instance: > >>> > >>> int main() { > >>> int i = 0; > >>> switch(i) {} > >>> } > >>> > >>> yields no warnings in clang with -Wunreachable-code (or -Wall), but at > >>> /W3 in MSVC you get: warning C4060: switch statement contains no > >>> 'case' or 'default' labels. Will it catch lots of bugs? Probably > >>> not. But it also should have basically no false positives either. It > >>> would be nice for people who want their code to also be warning-free > >>> under MSVC. > >> > >> > >> I think we need more justification for adding the warning than this. > >> > >> It doesn't seem reasonable to expect us to implement the union of all > >> warnings which every existent compiler produces. Some of those warnings > are > >> really dumb. For instance, /W3 also enables C4800, which I don't think > we > >> will ever want to implement. > > > > > > I agree that warning (forcing value to bool) is really, really bad, and I > > hope most serious projects turn it off. > > > > However, I can say it's pretty annoying if you're working on > cross-platform > > project and have to push to another platform to build just to get some > > compiler warnings for a compiler which you're expected to keep warning > > clean, value judgements about its diagnostic quality aside. That cycle > is > > long and painful, and if you skip it people often yell at you. > > > > While clang will never be able to paper over the differences between > > platforms, having some off-by-default diagnostics like this could save > users > > some time and increase the likelihood that their changes Just Work on the > > first push. >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
