pg.ddx.io pgsql-hackers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
Silencing the remaining clang 15 warnings 4+ messages / 2 participants [nested] [flat]
Commits from this thread: 2022-09-21 f6ce9cee62 b8464d2f5d Fix configure's AC_CHECK_DECLS tests to work correctly with clang. 2022-09-21 bb8dbc9f25 88c947cb52 9a2267bcfc c705646b75 96e595124e 3c5a33a52e bf21612344 850cb88e4b 9c69e2640f db8e36682d Suppress more variable-set-but-not-used warnings from clang 15. 2022-09-20 b7d9b0c266 f38a0bde21 85c077f9e0 7603087043 612e796612 2e124d857a ca3b730baa 152c9f7b8f 6ae8aee0b6 Suppress variable-set-but-not-used warnings from clang 15.
* Silencing the remaining clang 15 warnings @ 2022-09-19 19:20 Tom Lane <tgl@sss.pgh.pa.us> 2022-09-19 22:56 ` Re: Silencing the remaining clang 15 warnings Thomas Munro <thomas.munro@gmail.com> 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2022-09-19 19:20 UTC (permalink / raw) To: pgsql-hackers@lists.postgresql.org While working on the -Wdeprecated-non-prototype fixups discussed nearby, I saw that clang 15.0 produces a few other new warnings (which are also visible in the buildfarm). Pursuant to our usual policy that we should suppress warnings on compilers likely to be used for development, here's a patch to silence them. There are three groups of these: * With %pure-parser, Bison makes the "yynerrs" variable local instead of static, and then if you don't use it clang notices that it's set but never read. There doesn't seem to be a way to persuade Bison not to emit the variable at all, so here I've just added "(void) yynerrs;" to the topmost production of each affected grammar. If anyone has a nicer idea, let's hear it. * xlog.c's AdvanceXLInsertBuffer has a local variable "npages" that is only read in the "#ifdef WAL_DEBUG" stanza at the bottom. Here I've done the rather ugly and brute-force thing of wrapping all the variable's references in "#ifdef WAL_DEBUG". (I tried marking it PG_USED_FOR_ASSERTS_ONLY, but oddly that did not silence the warning.) I kind of wonder how useful this function's WAL_DEBUG output is --- maybe just dropping that altogether would be better? * array_typanalyze.c's compute_array_stats counts the number of null arrays in the column, but then does nothing with the result. AFAICS this is redundant with what std_compute_stats will do, so I just removed the variable. Any thoughts? regards, tom lane Attachments:^ permalink raw reply [nested|flat] 4+ messages in threadsilence-clang-15-warnings.patch (text/x-diff, 2.7K) download
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 81d339d57d..2651bcaa76 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1785,7 +1785,9 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, TimeLineID tli, bool opportunistic) XLogRecPtr NewPageEndPtr = InvalidXLogRecPtr; XLogRecPtr NewPageBeginPtr; XLogPageHeader NewPage; +#ifdef WAL_DEBUG int npages = 0; +#endif LWLockAcquire(WALBufMappingLock, LW_EXCLUSIVE); @@ -1928,7 +1930,9 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, TimeLineID tli, bool opportunistic) XLogCtl->InitializedUpTo = NewPageEndPtr; +#ifdef WAL_DEBUG npages++; +#endif } LWLockRelease(WALBufMappingLock); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 82f03fc9c9..c6ec694ceb 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -864,6 +864,7 @@ parse_toplevel: stmtmulti { pg_yyget_extra(yyscanner)->parsetree = $1; + (void) yynerrs; /* suppress compiler warning */ } | MODE_TYPE_NAME Typename { diff --git a/src/backend/utils/adt/array_typanalyze.c b/src/backend/utils/adt/array_typanalyze.c index 1964cedd93..2360c680ac 100644 --- a/src/backend/utils/adt/array_typanalyze.c +++ b/src/backend/utils/adt/array_typanalyze.c @@ -218,7 +218,6 @@ compute_array_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc, { ArrayAnalyzeExtraData *extra_data; int num_mcelem; - int null_cnt = 0; int null_elem_cnt = 0; int analyzed_rows = 0; @@ -320,8 +319,7 @@ compute_array_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc, value = fetchfunc(stats, array_no, &isnull); if (isnull) { - /* array is null, just count that */ - null_cnt++; + /* ignore arrays that are null overall */ continue; } diff --git a/src/backend/utils/adt/jsonpath_gram.y b/src/backend/utils/adt/jsonpath_gram.y index 7e28853a57..2a56629cc3 100644 --- a/src/backend/utils/adt/jsonpath_gram.y +++ b/src/backend/utils/adt/jsonpath_gram.y @@ -113,6 +113,7 @@ result: *result = palloc(sizeof(JsonPathParseResult)); (*result)->expr = $2; (*result)->lax = $1; + (void) yynerrs; } | /* EMPTY */ { *result = NULL; } ; diff --git a/src/bin/pgbench/exprparse.y b/src/bin/pgbench/exprparse.y index ade2ecdaab..f6387d4a3c 100644 --- a/src/bin/pgbench/exprparse.y +++ b/src/bin/pgbench/exprparse.y @@ -80,7 +80,10 @@ static PgBenchExpr *make_case(yyscan_t yyscanner, PgBenchExprList *when_then_lis %% -result: expr { expr_parse_result = $1; } +result: expr { + expr_parse_result = $1; + (void) yynerrs; /* suppress compiler warning */ + } elist: { $$ = NULL; } | expr { $$ = make_elist($1, NULL); }
* Re: Silencing the remaining clang 15 warnings 2022-09-19 19:20 Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> @ 2022-09-19 22:56 ` Thomas Munro <thomas.munro@gmail.com> 2022-09-19 23:06 ` Re: Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 4+ messages in thread From: Thomas Munro @ 2022-09-19 22:56 UTC (permalink / raw) To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org> On Tue, Sep 20, 2022 at 7:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > * With %pure-parser, Bison makes the "yynerrs" variable local > instead of static, and then if you don't use it clang notices > that it's set but never read. There doesn't seem to be a way > to persuade Bison not to emit the variable at all, so here I've > just added "(void) yynerrs;" to the topmost production of each > affected grammar. If anyone has a nicer idea, let's hear it. +1. FTR they know about this: https://www.mail-archive.com/bison-patches@gnu.org/msg07836.html https://github.com/akimd/bison/commit/a166d5450e3f47587b98f6005f9f5627dbe21a5b ... but that YY_ATTRIBUTE_UNUSED hasn't landed in my systems' /usr[/local]/share/bison/skeletons/yacc.c yet and it seems harmless and also legitimate to reference yynerrs from an action. > * xlog.c's AdvanceXLInsertBuffer has a local variable "npages" > that is only read in the "#ifdef WAL_DEBUG" stanza at the > bottom. Here I've done the rather ugly and brute-force thing > of wrapping all the variable's references in "#ifdef WAL_DEBUG". > (I tried marking it PG_USED_FOR_ASSERTS_ONLY, but oddly that > did not silence the warning.) I kind of wonder how useful this > function's WAL_DEBUG output is --- maybe just dropping that > altogether would be better? No opinion on the value of the message, but maybe pg_attribute_unused() would be better? > * array_typanalyze.c's compute_array_stats counts the number > of null arrays in the column, but then does nothing with the > result. AFAICS this is redundant with what std_compute_stats > will do, so I just removed the variable. +1 ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Silencing the remaining clang 15 warnings 2022-09-19 19:20 Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> 2022-09-19 22:56 ` Re: Silencing the remaining clang 15 warnings Thomas Munro <thomas.munro@gmail.com> @ 2022-09-19 23:06 ` Tom Lane <tgl@sss.pgh.pa.us> 2022-09-21 15:44 ` Re: Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 1 reply; 4+ messages in thread From: Tom Lane @ 2022-09-19 23:06 UTC (permalink / raw) To: Thomas Munro <thomas.munro@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org> Thomas Munro <thomas.munro@gmail.com> writes: > On Tue, Sep 20, 2022 at 7:20 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> * xlog.c's AdvanceXLInsertBuffer has a local variable "npages" >> that is only read in the "#ifdef WAL_DEBUG" stanza at the >> bottom. Here I've done the rather ugly and brute-force thing >> of wrapping all the variable's references in "#ifdef WAL_DEBUG". >> (I tried marking it PG_USED_FOR_ASSERTS_ONLY, but oddly that >> did not silence the warning.) I kind of wonder how useful this >> function's WAL_DEBUG output is --- maybe just dropping that >> altogether would be better? > No opinion on the value of the message, but maybe > pg_attribute_unused() would be better? I realized that the reason PG_USED_FOR_ASSERTS_ONLY didn't help is that it expands to empty in an assert-enabled build, which is what I was testing. So yeah, using pg_attribute_unused() directly would probably work better. (Also, I tried an assert-disabled build and found one additional new warning; same deal where clang doesn't believe "foo++;" is reason to consider foo to be used. That one, PG_USED_FOR_ASSERTS_ONLY can fix.) regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Silencing the remaining clang 15 warnings 2022-09-19 19:20 Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> 2022-09-19 22:56 ` Re: Silencing the remaining clang 15 warnings Thomas Munro <thomas.munro@gmail.com> 2022-09-19 23:06 ` Re: Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> @ 2022-09-21 15:44 ` Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2022-09-21 15:44 UTC (permalink / raw) To: Thomas Munro <thomas.munro@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org> HEAD and v15 now compile cleanly for me with clang 15.0.0, but I find that there's still work to do in the back branches: * There are new(?) -Wunused-but-set-variable warnings in every older branch, which we evidently cleaned up or rewrote at one point or another. I think this is definitely worth fixing in the in-support branches. I'm a bit less sure if it's worth the trouble in the out-of-support branches. * The 9.2 and 9.3 branches spew boatloads of 'undeclared library function' warnings about strlcpy() and related functions. This is evidently because 16fbac39f was only back-patched as far as 9.4. There are enough of these to be pretty annoying if you're trying to build those branches with clang, so I think this is clearly justified for back-patching into the older out-of-support branches, assuming that the patch will work there. (I see that 9.2 and 9.3 were still on the prior version of autoconf, so it might not be an easy change.) I also observe that 9.2 and 9.3 produce float.c:1278:29: warning: implicit conversion from 'int' to 'float' changes value from 2147483647 to 2147483648 [-Wimplicit-const-int-float-conversion] This is because cbdb8b4c0 was only back-patched as far as 9.4. However, I think that that would *not* be fit material for back-patching into out-of-support branches, since our policy for them is "no behavioral changes". regards, tom lane ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2022-09-21 15:44 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-09-19 19:20 Silencing the remaining clang 15 warnings Tom Lane <tgl@sss.pgh.pa.us> 2022-09-19 22:56 ` Thomas Munro <thomas.munro@gmail.com> 2022-09-19 23:06 ` Tom Lane <tgl@sss.pgh.pa.us> 2022-09-21 15:44 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox