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 <and...@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