On Sun, Feb 26, 2023 at 04:11:45PM -0500, Melanie Plageman wrote:
> 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.
Just kidding: it reproduces if the defined enum has two or less values.
Interesting...
After discovering this, tried out various solutions including one Andres
suggested:
for (IOOp io_op = 0; (int) io_op < IOOP_NUM_TYPES; io_op++)
and it does silence the warning. What do you think?
- Melanie