| From: | Peter Eisentraut <peter(at)eisentraut(dot)org> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Fix -Wshadow=local warnings |
| Date: | 2026-09-17 09:29:32 |
| Message-ID: | a75a6d38-ea88-4459-ad83-e7425aa03c9a@eisentraut.org |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 02.09.26 07:19, Chao Li wrote:
> A few small comments:
>
> 1 - 0001 - dependencies.c
> ```
> * expression into *expr.
> */
> static bool
> -dependency_is_compatible_expression(Node *clause, Index relid, List *statlist, Node **expr)
> +dependency_is_compatible_expression(Node *clause, Index relid, List *statlist, Node **stat_expr_p)
> ```
>
> As “expr” is renamed, the function header comment needs to be updated as well.
>
> 2 - 0001 - pg_constraint.c
> ```
> + CookedConstraint *cooked_constr;
> ```
>
> In the current RelationGetNotNullConstraints(), other local variables use camelCase naming, for example constrRel, so maybe it would be better to keep the naming style consistent.
Thanks, the patch with the code changes has been committed, with your
suggestions incorporated.
The patch to activate the option couldn't be committed yet because the
workaround for the LLVM headers didn't work and had to be reverted
(commit 7a0aa99e51c). I have worked on a new solution that works more
along the lines of how other per-file or per-module "-Wno-..." options
are handled. I think this will be simpler and less fragile. See
attached patch.
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Use-warning-option-Wshadow-local.patch | text/plain | 17.6 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-09-17 09:44:06 | Re: Row pattern recognition |
| Previous Message | Henri GASC | 2026-09-17 09:25:48 | Re: [SQL/PGQ] Native executor for Graph query |