From: | Greg Nancarrow <gregn4422(at)gmail(dot)com> |
---|---|
To: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Windows build warnings |
Date: | 2021-11-26 04:34:33 |
Message-ID: | CAJcOf-f_maFpPGqNJSjv7K76e8Hettf6PKTXOpH3ZJvJ5260Rg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Nov 25, 2021 at 11:03 PM Daniel Gustafsson <daniel(at)yesql(dot)se> wrote:
>
> To silence the warnings in the meantime (if the rework at all happens) we
> should either apply the patch from Greg or add C4101 to disablewarnings in
> src/tools/msvc/Project.pm as mentioned above. On top of that, we should apply
> the patch I proposed downthread to remove PG_USED_FOR_ASSERTS_ONLY where it's
> no longer applicable. Personally I'm fine with either, and am happy to make it
> happen, once we agree on what it should be.
>
AFAICS, the fundamental difference here seems to be that the GCC
compiler still regards a variable as "unused" if it is never read,
whereas if the variable is set (but not necessarily read) that's
enough for the Windows C compiler to regard it as "used".
This is why, at least for the majority of cases, why we're not seeing
the C4101 warnings on Windows where PG_USED_FOR_ASSERTS_ONLY has been
used in the Postgres source, because in those cases the variable has
been set prior its use in an Assert or "#ifdef USE_ASSERT_CHECKING"
block.
IMO, for the case in point, it's best to fix it by either setting the
variables to NULL, prior to their use in the "#ifdef
USE_ASSERT_CHECKING" block, or by applying my patch.
Of course, this doesn't address fixing the PG_USED_ONLY_FOR_ASSERTS
macro to work on Windows, but I don't see an easy way forward on that
if it's to remain in its "variable attribute" form, and in any case
the Windows C compiler doesn't seem to support any annotation to mark
a variable as potentially unused.
Personally I'm not really in favour of outright disabling the C4101
warning on Windows, because I think it is a useful warning for
Postgres developers on Windows for cases unrelated to the use of
PG_USED_FOR_ASSERTS_ONLY.
I agree with your proposal to apply your patch to remove
PG_USED_FOR_ASSERTS_ONLY where it's no longer applicable.
Regards,
Greg Nancarrow
Fujitsu Australia
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-11-26 04:45:09 | Re: Windows build warnings |
Previous Message | Amul Sul | 2021-11-26 04:24:30 | Re: prevent immature WAL streaming |