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

Compiler warnings with C++ compilation #77

Open
efiring opened this issue Nov 9, 2024 · 7 comments
Open

Compiler warnings with C++ compilation #77

efiring opened this issue Nov 9, 2024 · 7 comments
Assignees

Comments

@efiring
Copy link
Member

efiring commented Nov 9, 2024

C++ compilation triggered by the vast number of tests introduced in #71 and #75 yields a flood of warnings in the compilation logs. They come mostly from gsw_check_functions.c; I suspect one or two small changes in that file, repeated in many places, might clear up most of them, but I have not looked closely. There are so many compilations and logs that it is hard to keep track of which runs are clean and which show the warnings, but I think the warnings might be specific to the meson builds--for which the logs don't show the actual compiler invocations.

There are at least 2 questions here:

  1. Are there clean and unobtrusive code changes that will lead to warnings-free compilations in all cases of interest?
  2. Can we greatly reduce the number of test compilations without losing important information?
@peter-urban peter-urban self-assigned this Nov 11, 2024
@peter-urban
Copy link
Collaborator

peter-urban commented Nov 11, 2024

First investigation:

  1. The difficult to read warnings are generated in all cases where gcc or clang (or clang-cl on windows) are used to build the tests in c++ mode (the default was that c++ was only used for msvc compiler on windows)
  • I'll look into fixing these warnings;
  1. There are some additional warnings for the gcc compiler on linux. ("maybe used uninitialized") They could be caused by meson adding an additional compiler flag or defaulting to a different warning level. However I don't get them on my computer so this may also be related to the specific gcc version (11) as well
  • I'll investigate this when 1) is resolved
  1. Personally, I am a big fan of many test configurations. I made the experience that odd behavior between configurations can sometimes hint problems worth solving. But I agree: I overdid this here, the test configurations need to be simplified.
  • I'll look into this when I find some time. Suggestions welcome
  • As a first measure I'll remove two exotic configurations: macos-gcc and linux-clang. It is probably safe to assume that these compilers work similar enough on these somewhat similar systems. (and it seems that calling gcc on mac, actually calls clang anyways)

@peter-urban
Copy link
Collaborator

peter-urban commented Nov 11, 2024

Ok, I have more PRs in the pipeline to step-by-step resolve 1) and 2). However, they build upon #79 so I will wait until that one is merged.

@peter-urban
Copy link
Collaborator

@efiring responding to the comment in #80

We could modify the tests to run ninja -v directly (instead of meson compile or meson test)
The output would then look like this:

[1/6] gcc -Igsw_check.p -I. -I.. -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -O3 -MD -MQ gsw_check.p/gsw_check_functions.c.o -MF gsw_check.p/gsw_check_functions.c.o.d -o gsw_check.p/gsw_check_functions.c.o -c ../gsw_check_functions.c
[2/6] gcc -Ilibgswteos-10.so.p -I. -I.. -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -O3 -fPIC -MD -MQ libgswteos-10.so.p/gsw_saar.c.o -MF libgswteos-10.so.p/gsw_saar.c.o.d -o libgswteos-10.so.p/gsw_saar.c.o -c ../gsw_saar.c
[3/6] gcc -Ilibgswteos-10.so.p -I. -I.. -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -O3 -fPIC -MD -MQ libgswteos-10.so.p/gsw_oceanographic_toolbox.c.o -MF libgswteos-10.so.p/gsw_oceanographic_toolbox.c.o.d -o libgswteos-10.so.p/gsw_oceanographic_toolbox.c.o -c ../gsw_oceanographic_toolbox.c
[4/6] gcc  -o libgswteos-10.so libgswteos-10.so.p/gsw_oceanographic_toolbox.c.o libgswteos-10.so.p/gsw_saar.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 -shared -fPIC -Wl,-soname,libgswteos-10.so -lm
[5/6] /ssd/opt/miniforge3/envs/dev/bin/meson --internal symbolextractor /ssd/src/GSW-C-meson/builddir libgswteos-10.so libgswteos-10.so libgswteos-10.so.p/libgswteos-10.so.symbols 
[6/6] gcc  -o gsw_check gsw_check.p/gsw_check_functions.c.o -Wl,--as-needed -Wl,--no-undefined -Wl,-O1 '-Wl,-rpath,$ORIGIN/' -Wl,-rpath-link,/ssd/src/GSW-C-meson/builddir/ -Wl,--start-group libgswteos-10.so -lm -Wl,--end-group

This is after #80, otherwise you'd see the -Wall and -Wextra

Would you prefer this output? If so, I'll create a PR

@peter-urban
Copy link
Collaborator

#82 and #83 should resolve the remaining warnings

@peter-urban
Copy link
Collaborator

peter-urban commented Nov 12, 2024

Thx for merging. If I didn't miss anything, the warnings are now resolved.
Next step in this issue would be to further simplify the test matrix.

a) One possibility would be to not test shared and static libraries for all configurations.
Maybe shared is enough for most cases? Static will likely work if shared does.

b) Additionally: renaming the test cases may help to increase the overview. Suggestions welcome.

I'll look into these 2 steps, but I'll first have to focus on other projects again, so it may take some time.

@peter-urban
Copy link
Collaborator

Additionally, we could consider forcing the compiler to treat warnings as errors?
This way we wouldn't need to check the logs, unless there is a failure?

@efiring
Copy link
Member Author

efiring commented Nov 12, 2024

No rush at all for additional changes.
Mostly static only or shared only: I think this makes sense.
Warnings as errors: That would probably work out OK, given
that we are starting with no warnings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants