[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Monotone-devel] Mingw 64 bit build
From: |
Stephen Leake |
Subject: |
Re: [Monotone-devel] Mingw 64 bit build |
Date: |
Mon, 05 May 2014 19:15:58 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (windows-nt) |
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
- [Monotone-devel] Mingw 64 bit build, Stephen Leake, 2014/05/03
- Re: [Monotone-devel] Mingw 64 bit build, Markus Wanner, 2014/05/05
- Re: [Monotone-devel] Mingw 64 bit build,
Stephen Leake <=
- Re: [Monotone-devel] Mingw 64 bit build, Markus Wanner, 2014/05/06
- Re: [Monotone-devel] Mingw 64 bit build, Stephen Leake, 2014/05/07
- Re: [Monotone-devel] Mingw 64 bit build, Stephen Leake, 2014/05/16
- Re: [Monotone-devel] Mingw 64 bit build, Markus Wanner, 2014/05/16
- Re: [Monotone-devel] Mingw 64 bit build, Stephen Leake, 2014/05/16