Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

op.c: recent change triggers two new build-time warnings #22614

Open
jkeenan opened this issue Sep 20, 2024 · 1 comment
Open

op.c: recent change triggers two new build-time warnings #22614

jkeenan opened this issue Sep 20, 2024 · 1 comment
Labels
build-time-warnings Replaces [META] Build-time warnings RT #133556 Needs Triage

Comments

@jkeenan
Copy link
Contributor

jkeenan commented Sep 20, 2024

When compiling blead (or perl-5.41.4) with clang-14 on linux, I am observing two previously unobserved build-time warnings. Both are coming from a recently modified line in op.c. One occurs during the compilation of opmini.c; the other during compilation of op.c. From output of make test_prep:

   1 echo @`sh  cflags "optimize='-O2'" opmini.o`  -DPERL_IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   2 @clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/inclu     de -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=     vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -DPERL     _IS_MINIPERL -DPERL_EXTERNAL_GLOB opmini.c
   3 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
   4     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
   5                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   6 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
   7 #define MAX(a,b) (((a)>(b))?(a):(b))
   8                     ~ ^ ~
   9 1 warning generated.
...
 498 clang-14 -c -DPERL_CORE -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/includ     e -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c99 -O2 -Wall -Werror=pointer-arith -Werror=v     la -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings op.c
 499 op.c:6096:23: warning: comparison of integers of different signs: 'unsigned long' and 'int' [-W     sign-compare]
 500     int max_end_len = MAX(STRLENs(INFTY), TOTAL_LEN(array[upper] - 1));
 501                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 502 /usr/include/x86_64-linux-gnu/sys/param.h:103:23: note: expanded from macro 'MAX'
 503 #define MAX(a,b) (((a)>(b))?(a):(b))
 504                     ~ ^ ~
 505 1 warning generated.

I haven't done a full bisection, but the line in question was most recently modified in 8038d05.

commit 8038d05097cbe358d957e77fa7fb1c96cdf0a900
Author:     Karl Williamson <[email protected]>
AuthorDate: Fri Aug 16 11:52:49 2024 -0600
Commit:     Karl Williamson <[email protected]>
CommitDate: Mon Sep 2 10:47:07 2024 -0600

    op.c: Improve invmap_dump output

@khwilliamson, can you take a look? Thanks.

@jkeenan jkeenan added Needs Triage build-time-warnings Replaces [META] Build-time warnings RT #133556 labels Sep 20, 2024
@Leont
Copy link
Contributor

Leont commented Sep 20, 2024

I would also like to note that MAX evaluates the higher of its arguments twice, which may be less than ideal for that TOTAL_LEN(array[upper] - 1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-time-warnings Replaces [META] Build-time warnings RT #133556 Needs Triage
Projects
None yet
Development

No branches or pull requests

2 participants