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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.