zuo: fix some compiler warnings #4971
Conversation
I often compile with a CFLAGS that looks like
-Wall -Wextra -Wstrict-prototypes -Wold-style-definition -Wshadow -Wpointer-arith -Wcast-qual -pedantic -O2 -std=c11
(I actually typically include -Wmissing-prototypes but Zuo produces many
warnings from that which are probably harmless.) These changes cover the
generated warnings (mostly shadowed variables, unused function
arguments, and missing void parameter lists). Here's a selection of
interesting cases:
- zuo_read_fail{,2} takes an "s" that is never used. That did _not_
appear to be the result of compliance with a prototype whose
implementation was spread across ifdefs, so unless there's some stack
magic actually reading it, we can remove it. Similar arguments apply
to zuo_envvars_block's who parameter.
- dispatch_primitive0 _presumably_ uses args when invoking the proc, but
it might be reliance on stack magic? In any case, the (void)args;
statement marks them used for the compiler (as in the other cases),
but the (void) parameter list in the prototype of the case may be
inappropriate? Idem. for zuo_primitive0.
- zuo_make_void obviously ignores any arguments (like Racket's void); by
contrast, zuo_shell_to_strings_c needs to conform to a common
prototype even when all parameters are not needed, so we mark those as
"used". (The (void) cast idiom is essentially a no-op at any
optimization level.)
- a few const qualifiers were reintroduced where a cast would have
dropped them. I am not certain this is correct, but the prototypes of
the procedures called with the cast values seemed to expect the
constness.
./zuo build.zuo check passes after all these changes are compiled.
Here's the remaining warning:
zuo.c:979:21: warning: comparison of integers of different signs: 'zuo_int32_t' (aka 'int') and 'unsigned long long' [-Wsign-compare]
for (i = 0; i < len / sizeof(zuo_int32_t); i++)
~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~
My understanding of C and integer arithmetic is woefully incomplete, so
I'm not sure if casting (on which side of <?) is appropriate or if there
is a better solution.
|
Thanks! These changes look like a good idea to me. I agree about When I tried compiling with the flags you list, I saw many more warnings on various platforms, so I've pushed another commit to address those. Warnings from |
|
Thanks!
|
|
Maybe it would be worth adding a CI action to test these flags, either here or in the Zuo repository. |
|
I'll push a first draft of a CI workflow for that when I get an internet connection for my laptop. It could use some improvements (such as running on a matrix of build platforms). |
This is based on the compiler flags and checks used for 8ae70ca (zuo: fix some compiler warnings, 2024-04-14). There's probably room for more CI for Zuo (_e.g._, different platforms, ./configure + make, etc.). Suggested by Philip McGrath <philip@philipmcgrath.com>
|
I think CI is better (only) at the https://github.com/racket/zuo repo. I'd like to merge this change there, first, anyway. @benknoble, I could create a new PR at the racket/zuo repo, but I'll leave it to you if you'd prefer to create it. Let me know. |
|
Ah, I misunderstood the preferred direction. I will get this opened over there instead. |
shhyou
added
ci
I figured a patch gave a better starting point for discussion, but I admit I'm not confident in some of these changes and I'm opening this mostly to generate conversation.
For the unused parameters, I considered introducing
but didn't know if that fit the project style. I certainly didn't think we want to deal with compiler-specific attributes like
__unused.I often compile with a CFLAGS that looks like
-Wall -Wextra -Wstrict-prototypes -Wold-style-definition -Wshadow -Wpointer-arith -Wcast-qual -pedantic -O2 -std=c11
(I actually typically include -Wmissing-prototypes but Zuo produces many warnings from that which are probably harmless.) These changes cover the generated warnings (mostly shadowed variables, unused function arguments, and missing void parameter lists). Interesting cases are in the commit message.
./zuo build.zuo check passes after all these changes are compiled.
Here's the remaining warning:
My understanding of C and integer arithmetic is woefully incomplete, so I'm not sure if casting (on which side of <?) is appropriate or if there is a better solution.