On Sun, Feb 26, 2023 at 12:33:03PM -0800, Andres Freund wrote:
> On 2023-02-26 15:08:33 -0500, Tom Lane wrote:
> > Andres Freund <andres@anarazel.de> writes:
> > > They're all animals for testing older LLVM versions. They're using
> > > pretty old clang versions. phycodurus and dragonet are clang 3.9, petalura and
> > > desmoxytes is clang 4, idiacanthus and pogona are clang 5.
> >
> > [ shrug ... ] If I thought this was actually good code, I might
> > agree with ignoring these warnings; but I think what it mostly is
> > is misleading overcomplication.
>
> I don't mind removing *_FIRST et al by using 0. None of the proposals for
> getting rid of *_NUM_* seemed a cure actually better than the disease.
I am also fine with removing *_FIRST and allowing those electrons to
move on to bigger and better things :)
>
> Adding a cast to int of the loop iteration variable seems to work and only
> noticeably, not untollerably, ugly.
>
> One thing that's odd is that the warnings don't appear reliably. The
> "io_op < IOOP_NUM_TYPES" comparison in pgstatfuncs.c doesn't trigger any
> with clang-4.
Using an int and casting all over the place certainly doesn't make the
code more attractive, but I am fine with this if it seems like the least
bad solution.
I didn't want to write a patch with this (ints instead of enums as loop
control variable) without being able to reproduce the warnings myself
and confirm the patch silences them. However, I wasn't able to reproduce
the warnings myself. I tried to do so with a minimal repro on godbolt,
and even with
-Wtautological-constant-out-of-range-compare -Wall -Wextra -Weverything -Werror
I couldn't get clang 4 or 5 (or a number of other compilers I randomly
picked from the dropdown) to produce the warnings.
- Melanie