Re: [Monotone-devel] Mingw 64 bit build

[Stephen Leake]

Markus Wanner <address@hidden> 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