Re: Mingw 64 bit build
Stephen Leake <[email protected]>
| Newsgroups | gmane.comp.version-control.monotone.devel |
|---|---|
| Message-ID | <[email protected]> |
Markus Wanner <[email protected]> writes: > Stephen, > > On 05/04/2014 03:48 PM, Stephen Leake wrote: >>> I just tried: There are a couple of places where Unix needs an argument >>> that you've commented out. Please revert those. >> >> Done in nvm.cleanup-warnings. > > No, I still found lots crufty casts to void like this one: > > @@ -2384,6 +2457,10 @@ CMD_AUTOMATE(get_workspace_root, "", > "", > options::opts::none) > { > + (void)execid; > + (void)output; > + (void)args; > + > workspace work(app); > output << get_current_working_dir() << '\n'; > } > > Without explanation, lots of readers of that code, who didn't happen to > follow this thread (or even myself in three years from now) will ask > themselves: WTF? (See multiple instances of that on stackoverflow.) We could add this idiom to HACKING if we keep it. But I agree it's best to not keep the warnings enabled. > Rather than adding even more clutter in the form of a comment about > compiler happiness or some such, let's please just keep that warning > disabled. It's gain-to-pain ratio is way too low. Apparently we disagree on that. All my work projects require that warning. > Note that we use "-Wno-unused" since 2006 (c1ecd781). > The only thing that's "new" is that gcc 4.8 now emits that "unused > parameter" warning even though "-Wno-unused" is given (my gcc 4.7 > doesn't do that). Ah; I misread that option; it's trying to _suppress_ the warnings, not enable them. > If you want to make an argument about enabling warnings on unused things > in general, please try dropping the "-Wno-unused" and check the warnings > that arise. Yes, that would make sense. > I can imagine enabling some of the currently disabled gcc warnings. The > unused-but-set-* ones seem useful on the surface, for example. Consider > that AC_PROG_CXX_WARNINGS doesn't currently distinguish between gcc and > clang (3.4), though. And their set of supported options certainly > differs slightly. I tried enabling -Wusused-but-set-variable, but clang > doesn't understand that, for example. Also mind older versions... > Overall, to me this just doesn't seem worth the trouble. Ok. > And generally speaking, there are unused things which may become useful > at some point in time. I don't feel confident deleting all of those > things. After all, you didn't delete the parameter name for the very > same reason, but commented it out: You wanted to keep the name there, in > case it becomes useful. No, I kept it there so the implementation corresponds to the spec. > None the less, I also fixed a couple "unused-variable" and > "unused-local-typedef" warnings in a9efe468. Those were obvious > left-overs and useful hints from the compiler. But manually selected and > checked; I intentionally left other things in there, which some compiler > thinks are unused, but didn't seem useless to me. Yes, g++ doesn't always get it right; that's an argument in favor of suppressing the warning. > I hope this still satisfies your needs and allows you to "work much > better when there are no spurious warnings". Yes, as long as there are no warnings in the compiler output, my workflow works :). We should add something about this in HACKING, and perhaps suggest compiling new code with -Wunused enabled, to catch bugs before they get too far. -- -- Stephe