From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Tree-walker callbacks vs -Wdeprecated-non-prototype |
Date: | 2022-09-17 01:08:02 |
Message-ID: | 3953550.1663376882@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
clean-up-tree-walker-warnings-wip.patch | text/x-diff | 45.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-09-17 01:20:56 | Re: Making C function declaration parameter names consistent with corresponding definition names |
Previous Message | bt22nakamorit | 2022-09-17 00:44:33 | Re: Make ON_ERROR_STOP stop on shell script failure |