On Tue, Aug 30, 2022 at 3:14 AM David Rowley <dgrowleyml@gmail.com> wrote:
> I'm not really sure either. I tried compiling with clang 12.0.1 with
> -Wtautological-constant-out-of-range-compare and don't get this
> warning.
I have a much older clang version, it seems. clang -v reports 5.0.2. I
use -Wall and -Werror as a matter of habit. It looks like 5.0.2 was
released in May 2018, installed by me in November of 2019, and I just
haven't had a reason to upgrade.
> I think the Assert is useful as if we were ever to add an enum member
> with the value of 8 and forgot to adjust MEMORY_CONTEXT_METHODID_BITS
> then bad things would happen inside MemoryChunkSetHdrMask() and
> MemoryChunkSetHdrMaskExternal(). I think it's unlikely we'll ever get
> that many MemoryContext types, but I don't know for sure and would
> rather the person who adds the 9th one get alerted to the lack of bit
> space in MemoryChunk as soon as possible.
Well I don't have a problem with that, but I think we should try to do
it without causing compiler warnings. The attached patch fixes it for
me.
> As much as I'm not a fan of adding new warnings for compiler options
> that are not part of our standard set, I feel like if there are
> warning flags out there that are as giving us false warnings such as
> this one, then we shouldn't trouble ourselves trying to get rid of
> them, especially so when they force us to remove something which might
> catch a future bug.
For me the point is that, at least on the compiler that I'm using, the
warning suggests that the compiler will optimize the test away
completely, and therefore it wouldn't catch a future bug. Could there
be compilers where no warning is generated but the assertion is still
optimized away?
I don't know, but I don't think a 4-year-old compiler is such a fossil
that we shouldn't care whether it produces warnings. We worry about
operating systems and PostgreSQL versions that are almost extinct in
the wild, so saying we're not going to worry about failing to update
the compiler regularly enough within the lifetime of one off-the-shelf
MacBook does not really make sense to me.
--
Robert Haas
EDB: http://www.enterprisedb.com
