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

[C++ Headers] Non-sensical auto-generated if-statement leads to compilation error when compiled with -Werror #271

Closed
aentinger opened this issue Jan 24, 2023 · 5 comments
Labels
help wanted Extra attention is needed trivial

Comments

@aentinger
Copy link

As the compiler says, a uint8_t can not have an larger value than 255, consequently the if statement is dead code.

Compiler error

reg/udral/service/battery/Status_0_2.hpp:462:26: error: comparison is always false due to limited range of data type [-Werror=type-limits]
  462 |             if ( _size0_ > 255U)

reg/udral/service/battery/Status_0_2.hpp

        const std::uint8_t _size0_ = in_buffer.getU8(8U);
        in_buffer.add_offset(8U);
            if ( _size0_ > 255U)
@pavel-kirienko
Copy link
Member

The code is, indeed, suboptimal, but not incorrect, while your build process is somewhat misconfigured. Third-party code should not be processed with strict error settings, as it makes the build unnecessarily fragile and prone to failure like in your case. You should modify your build recipe to include Nunavut-generated headers as system headers to fix the problem. For GCC and clang, the correct flag is -isystem, which replaces -I.

@pavel-kirienko pavel-kirienko added help wanted Extra attention is needed trivial labels Jan 24, 2023
@aentinger
Copy link
Author

I'm only partially convinced. If OpenCyphal targets reliable systems its to be expected that whole codebase (including auto-generated code) does not break when using -Werror. This error may be trivial, but others may be not.

@pavel-kirienko
Copy link
Member

The matter of -Werror is orthogonal to the integrity of the codebase. Compilers are known to occasionally produce false diagnostics, which are promoted to build-breaking errors when strict compilation settings are given. This causes the compiler to reject valid code. It is, generally speaking, impossible to produce any non-trivial code that is certain to emit no diagnostics from all existing standard-compliant compilers. It follows that:

  1. -Werror and similar strict options should only be used with the code you control directly and never be applied to third-party code.
  2. Build error with -Werror does not indicate that the code is invalid or otherwise unacceptable.

@thirtytwobits
Copy link
Member

I'm inclined to accept this bug. I don't like normalizing deviance and if we can generate code that avoid well-known compiler warnings we should.

thirtytwobits added a commit that referenced this issue Mar 7, 2023
- #288 : Updating dev container documentation to point to ghcr.
- #283 : Detecting fixed boolean arrays and modifying c++ to emit
std::bitset
- #271 : Changing the type used when comparing array lengths to avoid
compiler warnings.
@thirtytwobits
Copy link
Member

This should be fixed with 2.0.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed trivial
Projects
None yet
Development

No branches or pull requests

3 participants