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

Check your code with werror and wpedantic #77

Closed
victimsnino opened this issue Mar 27, 2023 · 5 comments · Fixed by #76
Closed

Check your code with werror and wpedantic #77

victimsnino opened this issue Mar 27, 2023 · 5 comments · Fixed by #76
Labels
bug:confirmed Something isn't working (confirmed)
Milestone

Comments

@victimsnino
Copy link

victimsnino commented Mar 27, 2023

Hey! Thank you for this amazing testing framework. Currently I’m trying to add it to my library, but CI gets a bunch of error on your library due to werror and wpedantic flags

for example
Error: /home/runner/work/ReactivePlusPlus/ReactivePlusPlus/build/_deps/snitch-src/src/snitch.cpp:76:55: error: format string is not a string literal [-Werror,-Wformat-nonliteral]
60
const int return_code = std::snprintf(nullptr, 0, get_format_code(), value);

Or some stupid one

Error: /home/runner/work/ReactivePlusPlus/ReactivePlusPlus/build/_deps/snitch-src/include/snitch/snitch.hpp:1151:2: error: extra ‘;’ [-Werror=pedantic]
66
1151 | }; // namespace event

Full run can be found there: https://github.com/victimsnino/ReactivePlusPlus/actions/runs/4536738324

I could find workaround, but anyway it is better for you to have zero warnings even on most aggressive level :)

@cschreib
Copy link
Member

Hi there and thank you for the bug report. I am a bit confused as to why I am not catching these warnings, because I do compile the code with -Wall -Wextra -Werror. I don't use -pedantic, but I just tried adding it to the compilation options, and I am still not getting the warnings you are seeing. In case it was something wrong with my CMake scripts, I just copied snitch_all.hpp into Godbolt and explicitly set these warning levels, both with the latest clang and gcc; still no error.

Are you using a specific compiler version, and/or a specific platform? Or perhaps some additional compilation options. Are you able to create a minimal example I can reproduce, outside of your CI scripts? Having the full compilation command would also help.

At any rate, the warning about the extra semicolon is genuine, and I will fix it. The other warning is noise; I may be able to find an alternative implementation that does not trigger it, but that first requires me being able to reproduce it.

@cschreib cschreib added the bug:unconfirmed Something isn't working (to be confirmed) label Mar 28, 2023
@cschreib cschreib added this to the v1.1 milestone Mar 28, 2023
@victimsnino
Copy link
Author

I'm catching these errors during linking against snitch::snitch target, not header-only version. Most of the errors came from snitch.cpp file. Try this one. If you still can't reproduce, i would try to prepare example

cschreib added a commit that referenced this issue Mar 28, 2023
@cschreib
Copy link
Member

g++-10 -std=c++20 -Wall -Wextra -Werror -pedantic snitch.cpp -o snitch.o -I ../include -I ../build/

Still nothing 🤷

@cschreib
Copy link
Member

cschreib commented Mar 28, 2023

Correction, I do get

../include/snitch/snitch.hpp:1701:2: error: extra ‘;’ [-Werror=pedantic]
 1701 | }; // namespace event
      |  ^

I had already fixed it on my branch, hence why it wasn't showing up... Weirdly, it shows up only when compiling with g++-10, and not g++-11! Anyway, that one will be fixed.

For the rest of the warnings, I looked at your CI scripts, you specify more warnings on the command line: -Wall -Werror -Wextra -Wpedantic -Wcast-qual -Wformat=2 -Wundef -Werror=float-equal. If I use this, then I get

In file included from snitch.cpp:1:
../include/snitch/snitch.hpp: In function ‘constexpr snitch::impl::float_bits snitch::impl::to_bits(float)’:
../include/snitch/snitch.hpp:647:11: error: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Werror=float-equal]
  647 |     if (f != f) {
      |         ~~^~~~
../include/snitch/snitch.hpp:652:18: error: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Werror=float-equal]
  652 |     } else if (f == std::numeric_limits<float>::infinity()) {
      |                ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/snitch/snitch.hpp:657:18: error: comparing floating-point with ‘==’ or ‘!=’ is unsafe [-Werror=float-equal]
  657 |     } else if (f == -std::numeric_limits<float>::infinity()) {
      |                ~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
snitch.cpp: In instantiation of ‘bool {anonymous}::append_fmt(snitch::small_string_span, T) [with T = const void*; snitch::small_string_span = snitch::small_vector_span<char>]’:
snitch.cpp:119:34:   required from here
snitch.cpp:76:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
   76 |     const int return_code = std::snprintf(nullptr, 0, get_format_code<T>(), value);
      |                             ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
snitch.cpp: In instantiation of ‘bool {anonymous}::append_fmt(snitch::small_string_span, T) [with T = long unsigned int; snitch::small_string_span = snitch::small_vector_span<char>]’:
snitch.cpp:124:28:   required from here
snitch.cpp:76:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
snitch.cpp: In instantiation of ‘bool {anonymous}::append_fmt(snitch::small_string_span, T) [with T = long int; snitch::small_string_span = snitch::small_vector_span<char>]’:
snitch.cpp:128:28:   required from here
snitch.cpp:76:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
snitch.cpp: In instantiation of ‘bool {anonymous}::append_fmt(snitch::small_string_span, T) [with T = float; snitch::small_string_span = snitch::small_vector_span<char>]’:
snitch.cpp:132:28:   required from here
snitch.cpp:76:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]
snitch.cpp: In instantiation of ‘bool {anonymous}::append_fmt(snitch::small_string_span, T) [with T = double; snitch::small_string_span = snitch::small_vector_span<char>]’:
snitch.cpp:136:28:   required from here
snitch.cpp:76:42: error: format not a string literal, argument types not checked [-Werror=format-nonliteral]

Seems like they are coming from -Wformat=2 and -Werror=float-equal. These aren't included in -Wall -Wextra -pedantic, which is already quite a conservative set of warnings. If you want to enable these warnings, would suggest silencing them when building/including snitch. I'm afraid I can't support any and all combinations of warnings, particularly the non-standard ones.

@victimsnino
Copy link
Author

victimsnino commented Mar 28, 2023

yeah, sure, no problem. It is not blocker for me, just wanted to notify you about possible issue :). In this case I would suggest you to add (or at least, to mention)

target_compile_options(snitch PRIVATE "-w")

as part of the "installation guide"

@cschreib cschreib added bug:confirmed Something isn't working (confirmed) and removed bug:unconfirmed Something isn't working (to be confirmed) labels May 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:confirmed Something isn't working (confirmed)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants