Re: [Monotone-devel] Mingw 64 bit build

[Markus Wanner]

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

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.


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


Here are some warning counts for recent revs in nvm.cleanup-warnings.
All from my Debian testing box; number of warnings for these compilers
in the following order: gcc 4.7.3, gcc 4.8.2, clang 3.4; n/a means I
didn't check.

f2f237f5.. (your last commit):       0    7   156
491b5798.. (I merged with nvm):     n/a   7   156
882e1fd0.. (add -Wno-unused-param):  0    7   154
1b013c9f.. (revert casts to void):  n/a   7   148 (note the drop!)
a9efe468.. (cleanup gcc warn):      n/a   0   148
bfc3656a.. (cleanup clang warn):     0    0    0


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.

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.


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.


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.

I hope this still satisfies your needs and allows you to "work much
better when there are no spurious warnings".

Regards

Markus Wanner

signature.asc
Description: OpenPGP digital signature