Thread

Commits

  1. Remove accidentally added meson.build

  2. Disable clang 16's -Wcast-function-type-strict.

  3. Disable -Wdeprecated-non-prototype in the back branches.

  4. Revise tree-walk APIs to improve spec compliance & silence warnings.

  5. Future-proof the recursion inside ExecShutdownNode().

  1. Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-05-01T23:41:20Z

    Hi,
    
    As visible on seawasp (and noticed here in passing, while hacking on
    the opaque pointer changes for bleeding edge LLVM), Clang 15 now warns
    by default about our use of tree walkers functions with no function
    prototype, because the next revision of C (C23?) will apparently be
    harmonising with C++ in interpreting f() to mean f(void), not
    f(anything goes).
    
    nodeFuncs.c:2051:17: warning: passing arguments to a function without
    a prototype is deprecated in all versions of C and is not supported in
    C2x [-Wdeprecated-non-prototype]
                            return walker(((WithCheckOption *)
    node)->qual, context);
    
    Discussion trail:
    
    https://reviews.llvm.org/D123456
    https://discourse.llvm.org/t/rfc-enabling-wstrict-prototypes-by-default-in-c/60521
    http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2841.htm
    
    Not sure where to see the official status of N2841 (other than waiting
    for the next draft to pop out), but on random/unofficial social media
    I saw that it was accepted in February, and the Clang people
    apparently think it's in and I also saw a rumour that bleeding edge
    GCC takes this view if you run with -std=c2x (not tested by me).
    
    
    
    
  2. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-05-02T00:02:45Z

    Thomas Munro <thomas.munro@gmail.com> writes:
    > As visible on seawasp (and noticed here in passing, while hacking on
    > the opaque pointer changes for bleeding edge LLVM), Clang 15 now warns
    > by default about our use of tree walkers functions with no function
    > prototype, because the next revision of C (C23?) will apparently be
    > harmonising with C++ in interpreting f() to mean f(void), not
    > f(anything goes).
    
    Ugh.  I wonder if we can get away with declaring the walker arguments
    as something like "bool (*walker) (Node *, void *)" without having
    to change all the actual walkers to be exactly that signature.
    Having to insert casts in the walkers would be a major pain-in-the-butt.
    
    			regards, tom lane
    
    
    
    
  3. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-17T01:08:02Z

    I wrote:
    > Ugh.  I wonder if we can get away with declaring the walker arguments
    > as something like "bool (*walker) (Node *, void *)" without having
    > to change all the actual walkers to be exactly that signature.
    > Having to insert casts in the walkers would be a major pain-in-the-butt.
    
    No joy on that: both gcc and clang want the walkers to be declared
    as taking exactly "void *".
    
    Attached is an incomplete POC patch that suppresses these warnings
    in nodeFuncs.c itself and in costsize.c, which I selected at random
    as a typical caller.  I'll push forward with converting the other
    call sites if this way seems good to people.
    
    In nodeFuncs.c, we can hide the newly-required casts inside macros;
    indeed, the mutators barely need any changes because they already
    had MUTATE() macros that contained casts.  So on that side, it feels
    to me that this is actually a bit nicer than before.
    
    For the callers, we can either do it as I did below:
    
     static bool
    -cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
    +cost_qual_eval_walker(Node *node, void *ctx)
     {
    +	cost_qual_eval_context *context = (cost_qual_eval_context *) ctx;
    +
     	if (node == NULL)
     		return false;
    
    or perhaps like this:
    
     static bool
    -cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
    +cost_qual_eval_walker(Node *node, void *context)
     {
    +	cost_qual_eval_context *cqctx = (cost_qual_eval_context *) context;
    +
     	if (node == NULL)
     		return false;
    
    but the latter would require changing references further down in the
    function, so I felt it more invasive.
    
    It's sad to note that this exercise in hoop-jumping actually leaves
    us with net LESS type safety, because the outside callers of
    cost_qual_eval_walker are no longer constrained to call it with
    the appropriate kind of context struct.  Thanks, C committee.
    
    			regards, tom lane
    
    
  4. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-18T20:57:45Z

    I wrote:
    > Attached is an incomplete POC patch that suppresses these warnings
    > in nodeFuncs.c itself and in costsize.c, which I selected at random
    > as a typical caller.  I'll push forward with converting the other
    > call sites if this way seems good to people.
    
    Here's a fleshed-out patch that gets rid of all warnings of this sort
    (tested on clang version 15.0.0).
    
    While I remain happy enough with what has to be done in nodeFuncs.c,
    I'm really not happy at all with this point:
    
    > It's sad to note that this exercise in hoop-jumping actually leaves
    > us with net LESS type safety, because the outside callers of
    > cost_qual_eval_walker are no longer constrained to call it with
    > the appropriate kind of context struct.  Thanks, C committee.
    
    There are a lot of these walker/mutator functions and hence a whole
    lot of opportunity to pass the wrong thing, not only from the outer
    non-recursive call points but during internal recursions in the
    walkers/mutators themselves.
    
    I think we ought to seriously consider the alternative of changing
    nodeFuncs.c about like I have here, but not touching the walkers/mutators,
    and silencing the resulting complaints about function type casting by
    doing the equivalent of
    
    -    return expression_tree_walker(node, cost_qual_eval_walker,
    -                                  (void *) context);
    +    return expression_tree_walker(node,
    +                                  (tree_walker_callback) cost_qual_eval_walker,
    +                                  (void *) context);
    
    We could avoid touching all the call sites by turning
    expression_tree_walker and friends into macro wrappers that incorporate
    these casts.  This is fairly annoying, in that it gives up the function
    type safety the C committee wants to impose on us; but I really think
    the data type safety that we're giving up in this version of the patch
    is a worse hazard.
    
    BTW, I was distressed to discover that someone decided they could
    use ExecShutdownNode as a planstate_tree_walker() walker even though
    its argument list is not even the right length.  I'm a bit flabbergasted
    that we seem to have gotten away with that so far, because I'd have
    thought for sure that it'd break some platform's convention for which
    argument gets passed where.  I think we need to fix that, independently
    of what we do about the larger scope of these problems.  To avoid an
    API break, I propose making ExecShutdownNode just be a one-liner that
    calls an internal ExecShutdownNode_walker() function.  (I've not done
    it that way in the attached, though.)
    
    Thoughts?
    
    			regards, tom lane
    
    
  5. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-09-18T22:16:24Z

    On Mon, Sep 19, 2022 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > I think we ought to seriously consider the alternative of changing
    > nodeFuncs.c about like I have here, but not touching the walkers/mutators,
    > and silencing the resulting complaints about function type casting by
    > doing the equivalent of
    >
    > -    return expression_tree_walker(node, cost_qual_eval_walker,
    > -                                  (void *) context);
    > +    return expression_tree_walker(node,
    > +                                  (tree_walker_callback) cost_qual_eval_walker,
    > +                                  (void *) context);
    >
    > We could avoid touching all the call sites by turning
    > expression_tree_walker and friends into macro wrappers that incorporate
    > these casts.  This is fairly annoying, in that it gives up the function
    > type safety the C committee wants to impose on us; but I really think
    > the data type safety that we're giving up in this version of the patch
    > is a worse hazard.
    
    But is it defined behaviour?
    
    https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type
    
    > BTW, I was distressed to discover that someone decided they could
    > use ExecShutdownNode as a planstate_tree_walker() walker even though
    > its argument list is not even the right length.  I'm a bit flabbergasted
    > that we seem to have gotten away with that so far, because I'd have
    > thought for sure that it'd break some platform's convention for which
    > argument gets passed where.  I think we need to fix that, independently
    > of what we do about the larger scope of these problems.  To avoid an
    > API break, I propose making ExecShutdownNode just be a one-liner that
    > calls an internal ExecShutdownNode_walker() function.  (I've not done
    > it that way in the attached, though.)
    
    Huh... wouldn't systems that pass arguments right-to-left on the stack
    receive NULL for node?  That'd include the SysV i386 convention used
    on Linux, *BSD etc.  But that can't be right or we'd know about it...
    
    But certainly +1 for fixing that regardless.
    
    
    
    
  6. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-19T03:39:38Z

    Thomas Munro <thomas.munro@gmail.com> writes:
    > On Mon, Sep 19, 2022 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> ... This is fairly annoying, in that it gives up the function
    >> type safety the C committee wants to impose on us; but I really think
    >> the data type safety that we're giving up in this version of the patch
    >> is a worse hazard.
    
    > But is it defined behaviour?
    > https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type
    
    Well, what we're talking about is substituting "void *" (which is
    required to be compatible with "char *") for a struct pointer type.
    Standards legalese aside, that could only be a problem if the platform
    ABI handles "char *" differently from struct pointer types.  The last
    architecture I can remember dealing with where that might actually be
    a thing was the PDP-10.  Everybody has learned better since then, but
    the C committee is apparently still intent on making the world safe
    for crappy machine architectures.
    
    Also, if you want to argue that "void *" is not compatible with struct
    pointer types, then it's not real clear to me that we aren't full of
    other spec violations, because we sure do a lot of casting across that
    (and even more with this patch as it stands).
    
    I don't have the slightest hesitation about saying that if there's
    still an architecture out there that's like that, we won't support it.
    I also note that our existing code in this area would break pretty
    thoroughly on such a machine, so this isn't making it worse.
    
    			regards, tom lane
    
    
    
    
  7. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-09-19T04:32:59Z

    On Mon, Sep 19, 2022 at 3:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Thomas Munro <thomas.munro@gmail.com> writes:
    > > On Mon, Sep 19, 2022 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > >> ... This is fairly annoying, in that it gives up the function
    > >> type safety the C committee wants to impose on us; but I really think
    > >> the data type safety that we're giving up in this version of the patch
    > >> is a worse hazard.
    >
    > > But is it defined behaviour?
    > > https://stackoverflow.com/questions/559581/casting-a-function-pointer-to-another-type
    >
    > Well, what we're talking about is substituting "void *" (which is
    > required to be compatible with "char *") for a struct pointer type.
    > Standards legalese aside, that could only be a problem if the platform
    > ABI handles "char *" differently from struct pointer types.  The last
    > architecture I can remember dealing with where that might actually be
    > a thing was the PDP-10.  Everybody has learned better since then, but
    > the C committee is apparently still intent on making the world safe
    > for crappy machine architectures.
    >
    > Also, if you want to argue that "void *" is not compatible with struct
    > pointer types, then it's not real clear to me that we aren't full of
    > other spec violations, because we sure do a lot of casting across that
    > (and even more with this patch as it stands).
    >
    > I don't have the slightest hesitation about saying that if there's
    > still an architecture out there that's like that, we won't support it.
    > I also note that our existing code in this area would break pretty
    > thoroughly on such a machine, so this isn't making it worse.
    
    Yeah, I don't expect it to be a practical problem on any real system
    (that is, I don't expect any real calling convention to transfer a
    struct T * argument in a different place than void *).  I just wanted
    to mention that it's a new liberty.  It's one thing to cast struct T *
    to void * and back before dereferencing, and another to cast a pointer
    to a function that takes struct T * to a pointer to a function that
    takes void * and call it.  I considered proposing that myself when
    first reporting this problem, but fear of language lawyers put me off.
    
    
    
    
  8. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-19T04:53:30Z

    Thomas Munro <thomas.munro@gmail.com> writes:
    > On Mon, Sep 19, 2022 at 3:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> I also note that our existing code in this area would break pretty
    >> thoroughly on such a machine, so this isn't making it worse.
    
    > Yeah, I don't expect it to be a practical problem on any real system
    > (that is, I don't expect any real calling convention to transfer a
    > struct T * argument in a different place than void *).  I just wanted
    > to mention that it's a new liberty.
    
    No, it's not, because the existing coding here is already assuming that.
    The walker callbacks are generally declared as taking a "struct *"
    second parameter, but expression_tree_walker et al think they are
    passing a "void *" to them.  Even if a platform ABI had some weird
    special rule about how to call functions that you don't know the
    argument list for, it wouldn't fix this because the walkers sure do know
    what their arguments are.  The only reason this code works today is that
    in practice, "void *" *is* ABI-compatible with "struct *".
    
    I'm not excited about creating a demonstrable opportunity for bugs
    in order to make the code hypothetically more compatible with
    hardware designs that are thirty years obsolete.  (Hypothetical
    in the sense that there's little reason to believe there would
    be no other problems.)
    
    			regards, tom lane
    
    
    
    
  9. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-09-19T07:30:09Z

    On Mon, Sep 19, 2022 at 4:53 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Thomas Munro <thomas.munro@gmail.com> writes:
    > > On Mon, Sep 19, 2022 at 3:39 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > >> I also note that our existing code in this area would break pretty
    > >> thoroughly on such a machine, so this isn't making it worse.
    >
    > > Yeah, I don't expect it to be a practical problem on any real system
    > > (that is, I don't expect any real calling convention to transfer a
    > > struct T * argument in a different place than void *).  I just wanted
    > > to mention that it's a new liberty.
    >
    > No, it's not, because the existing coding here is already assuming that.
    > The walker callbacks are generally declared as taking a "struct *"
    > second parameter, but expression_tree_walker et al think they are
    > passing a "void *" to them.  Even if a platform ABI had some weird
    > special rule about how to call functions that you don't know the
    > argument list for, it wouldn't fix this because the walkers sure do know
    > what their arguments are.  The only reason this code works today is that
    > in practice, "void *" *is* ABI-compatible with "struct *".
    
    True.
    
    > I'm not excited about creating a demonstrable opportunity for bugs
    > in order to make the code hypothetically more compatible with
    > hardware designs that are thirty years obsolete.  (Hypothetical
    > in the sense that there's little reason to believe there would
    > be no other problems.)
    
    Fair enough.
    
    
    
    
  10. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-09-19T07:40:11Z

    On Mon, Sep 19, 2022 at 10:16 AM Thomas Munro <thomas.munro@gmail.com> wrote:
    > On Mon, Sep 19, 2022 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > BTW, I was distressed to discover that someone decided they could
    > > use ExecShutdownNode as a planstate_tree_walker() walker even though
    > > its argument list is not even the right length.  I'm a bit flabbergasted
    > > that we seem to have gotten away with that so far, because I'd have
    > > thought for sure that it'd break some platform's convention for which
    > > argument gets passed where.  I think we need to fix that, independently
    > > of what we do about the larger scope of these problems.  To avoid an
    > > API break, I propose making ExecShutdownNode just be a one-liner that
    > > calls an internal ExecShutdownNode_walker() function.  (I've not done
    > > it that way in the attached, though.)
    >
    > Huh... wouldn't systems that pass arguments right-to-left on the stack
    > receive NULL for node?  That'd include the SysV i386 convention used
    > on Linux, *BSD etc.  But that can't be right or we'd know about it...
    
    I take that back after looking up some long forgotten details; it
    happily ignores extra arguments.
    
    
    
    
  11. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-19T14:00:05Z

    Thomas Munro <thomas.munro@gmail.com> writes:
    > On Mon, Sep 19, 2022 at 10:16 AM Thomas Munro <thomas.munro@gmail.com> wrote:
    >> Huh... wouldn't systems that pass arguments right-to-left on the stack
    >> receive NULL for node?  That'd include the SysV i386 convention used
    >> on Linux, *BSD etc.  But that can't be right or we'd know about it...
    
    > I take that back after looking up some long forgotten details; it
    > happily ignores extra arguments.
    
    Yeah; the fact that no one has complained in several years seems to
    indicate that there's not a problem on supported platforms.  Still,
    unlike the quibbles over whether char and struct pointers are the
    same, it seems clear that this is the sort of inconsistency that
    C2x wants to forbid, presumably in the name of making the world
    safe for more-efficient function calling code.  So I think we'd
    better go fix ExecShutdownNode before somebody breaks it.
    
    Whichever way we jump on the tree-walker API changes, those won't
    be back-patchable.  I think the best we can do for the back branches
    is add a configure test to use -Wno-deprecated-non-prototype
    if available.  But the ExecShutdownNode change could be back-patched,
    and I'm leaning to doing so even though that breakage is just
    hypothetical today.
    
    			regards, tom lane
    
    
    
    
  12. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-19T18:10:12Z

    Here's a second-generation patch that fixes the warnings by inserting
    casts into a layer of macro wrappers.  I had supposed that this would
    cause us to lose all detection of wrongly-chosen walker functions,
    so I was very pleased to see this when applying it to yesterday's HEAD:
    
    execProcnode.c:792:2: warning: cast from 'bool (*)(PlanState *)' (aka 'bool (*)(struct PlanState *)') to 'planstate_tree_walker_callback' (aka 'bool (*)(struct PlanState *, void *)') converts to incompatible function type [-Wcast-function-type]
            planstate_tree_walker(node, ExecShutdownNode, NULL);
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    ../../../src/include/nodes/nodeFuncs.h:180:33: note: expanded from macro 'planstate_tree_walker'
            planstate_tree_walker_impl(ps, (planstate_tree_walker_callback) (w), c)
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    
    So we've successfully suppressed the pedantic -Wdeprecated-non-prototype
    warnings, and we have activated the actually-useful -Wcast-function-type
    warnings, which seem to do exactly what we want in this context:
    
    '-Wcast-function-type'
         Warn when a function pointer is cast to an incompatible function
         pointer.  In a cast involving function types with a variable
         argument list only the types of initial arguments that are provided
         are considered.  Any parameter of pointer-type matches any other
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
         pointer-type.  Any benign differences in integral types are
         ^^^^^^^^^^^^
         ignored, like 'int' vs.  'long' on ILP32 targets.  Likewise type
         qualifiers are ignored.  The function type 'void (*) (void)' is
         special and matches everything, which can be used to suppress this
         warning.  In a cast involving pointer to member types this warning
         warns whenever the type cast is changing the pointer to member
         type.  This warning is enabled by '-Wextra'.
    
    (That verbiage is from the gcc manual; clang seems to act the same
    except that -Wcast-function-type is selected by -Wall, or perhaps is
    even on by default.)
    
    So I'm pretty pleased with this formulation: no caller changes are
    needed, and it does exactly what we want warning-wise.
    
    			regards, tom lane
    
    
  13. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Robert Haas <robertmhaas@gmail.com> — 2022-09-19T18:11:40Z

    On Sun, Sep 18, 2022 at 4:58 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > BTW, I was distressed to discover that someone decided they could
    > use ExecShutdownNode as a planstate_tree_walker() walker even though
    > its argument list is not even the right length.  I'm a bit flabbergasted
    > that we seem to have gotten away with that so far, because I'd have
    > thought for sure that it'd break some platform's convention for which
    > argument gets passed where.  I think we need to fix that, independently
    > of what we do about the larger scope of these problems.  To avoid an
    > API break, I propose making ExecShutdownNode just be a one-liner that
    > calls an internal ExecShutdownNode_walker() function.  (I've not done
    > it that way in the attached, though.)
    
    I think this was brain fade on my part ... or possibly on Amit
    Kapila's part, but I believe it was probably me. I agree that it's
    impressive that it actually seemed to work that way.
    
    -- 
    Robert Haas
    EDB: http://www.enterprisedb.com
    
    
    
    
  14. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-09-20T16:15:13Z

    I wrote:
    > (That verbiage is from the gcc manual; clang seems to act the same
    > except that -Wcast-function-type is selected by -Wall, or perhaps is
    > even on by default.)
    
    Nah, scratch that: the reason -Wcast-function-type is on is that
    we explicitly enable it, and have done so since de8feb1f3 (v14).
    I did not happen to see this warning with gcc because the test runs
    I made with this patch already had c35ba141d, whereas I did my
    clang test on another machine that wasn't quite up to HEAD.
    So we should have good warning coverage for bogus walker signatures
    on both compilers.
    
    			regards, tom lane
    
    
    
    
  15. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-12-12T02:45:51Z

    As visible on seawasp and locally (16/main branch nightly packages),
    they decided to start warning about these casts with a new strict
    variant of the warning.  Their discussion:
    
    https://reviews.llvm.org/D134831
    
    There are also a few other cases unrelated to this thread's original
    problem, for example casts involving pg_funcptr_t, HashCompareFunc.  I
    guess our options would be to turn that warning off, or reconsider and
    try shoving the cast of "generic" arguments pointers down into the
    functions?
    
    
    
    
  16. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-12-12T03:07:05Z

    Thomas Munro <thomas.munro@gmail.com> writes:
    > As visible on seawasp and locally (16/main branch nightly packages),
    > they decided to start warning about these casts with a new strict
    > variant of the warning.  Their discussion:
    
    > https://reviews.llvm.org/D134831
    
    > There are also a few other cases unrelated to this thread's original
    > problem, for example casts involving pg_funcptr_t, HashCompareFunc.  I
    > guess our options would be to turn that warning off, or reconsider and
    > try shoving the cast of "generic" arguments pointers down into the
    > functions?
    
    I'm for "turn the warning off".  Per previous discussion, adhering
    strictly to that rule would make our code worse (less legible AND
    less safe), not better.
    
    			regards, tom lane
    
    
    
    
  17. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-12-12T03:43:41Z

    On Mon, Dec 12, 2022 at 4:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Thomas Munro <thomas.munro@gmail.com> writes:
    > > As visible on seawasp and locally (16/main branch nightly packages),
    > > they decided to start warning about these casts with a new strict
    > > variant of the warning.  Their discussion:
    >
    > > https://reviews.llvm.org/D134831
    >
    > > There are also a few other cases unrelated to this thread's original
    > > problem, for example casts involving pg_funcptr_t, HashCompareFunc.  I
    > > guess our options would be to turn that warning off, or reconsider and
    > > try shoving the cast of "generic" arguments pointers down into the
    > > functions?
    >
    > I'm for "turn the warning off".  Per previous discussion, adhering
    > strictly to that rule would make our code worse (less legible AND
    > less safe), not better.
    
    Alright, this seems to do the trick here.
    
  18. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Thomas Munro <thomas.munro@gmail.com> — 2022-12-13T01:18:48Z

    On Mon, Dec 12, 2022 at 4:43 PM Thomas Munro <thomas.munro@gmail.com> wrote:
    > On Mon, Dec 12, 2022 at 4:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > I'm for "turn the warning off".  Per previous discussion, adhering
    > > strictly to that rule would make our code worse (less legible AND
    > > less safe), not better.
    >
    > Alright, this seems to do the trick here.
    
    That did fix that problem.  But... seawasp also just recompiled its
    compiler and picked up new opaque pointer API changes.  So no green
    today.  I have more work to do to fix that, which might take some time
    to get back to.
    
    
    
    
  19. Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

    Andres Freund <andres@anarazel.de> — 2025-05-22T14:47:06Z

    Hi,
    
    On 2022-12-13 14:18:48 +1300, Thomas Munro wrote:
    > On Mon, Dec 12, 2022 at 4:43 PM Thomas Munro <thomas.munro@gmail.com> wrote:
    > > On Mon, Dec 12, 2022 at 4:07 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > > I'm for "turn the warning off".  Per previous discussion, adhering
    > > > strictly to that rule would make our code worse (less legible AND
    > > > less safe), not better.
    > >
    > > Alright, this seems to do the trick here.
    > 
    > That did fix that problem.  But... seawasp also just recompiled its
    > compiler and picked up new opaque pointer API changes.  So no green
    > today.  I have more work to do to fix that, which might take some time
    > to get back to.
    
    Presumably due to conflict resolution, this commit added an empty meson.build
    to REL_13_STABLE. I'll remove that...
    
    Greetings,
    
    Andres Freund