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. - Melanie
