Horton

Re: Tree-walker callbacks vs -Wdeprecated-non-prototype

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2022-09-19T18:10:12Z
Lists: pgsql-hackers

Attachments

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

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().