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

build: ensure full set of compiler warnings #58381

Open
mkschreder opened this issue May 29, 2023 · 4 comments
Open

build: ensure full set of compiler warnings #58381

mkschreder opened this issue May 29, 2023 · 4 comments
Assignees
Labels
area: Coding Guidelines Coding guidelines and style area: Toolchains Toolchains Enhancement Changes/Updates/Additions to existing features

Comments

@mkschreder
Copy link

mkschreder commented May 29, 2023

Is your enhancement proposal related to a problem? Please describe.

Currently zephyr does not enable all warnings. This means that some potential problems can be missed and also it means that full set of compiler warnings can not be enabled for all code in downstream projects.

Describe the solution you'd like
I would like to have all of the below warnings enabled and the specific lines with "-Wno..." removed once all issues with all subsystems have been fixed.

# treat all warnings as errors
zephyr_compile_options("-Werror")
# enable extra warnings
zephyr_compile_options("-Wextra")
# enable standard set of warnings (doesn't matter if already enabled)
zephyr_compile_options("-Wall")
# format options validation
zephyr_compile_options("-Wformat=2")
# more format options validation
zephyr_compile_options("-Wformat-truncation")
# detects some out of bounds array indices
zephyr_compile_options("-Warray-bounds")
# detects uninitialized variables
zephyr_compile_options("-Wuninitialized")
# detects passing null arguments into functions that do not check for it
zephyr_compile_options("-Wnonnull")

# Disable unused parameter warning
# Reason: zephyr pm functions fail to compile
zephyr_compile_options("-Wno-unused-parameter")

# Disable error on type limits (zephyr ASSERT)
# Reason: zephyr sys/assert does not compile
zephyr_compile_options("-Wno-type-limits")

# Disable error on comparison of integers of different signedness (zephyr)
# Reason: zephyr os/cbprintf does not compile
zephyr_compile_options("-Wno-sign-compare")

# Disable qualifiers (zephyr net_if.h)
zephyr_compile_options("-Wno-ignored-qualifiers")

# Disable error on missing field initializers
# Reason: sync_status in drivers/spi/spi_context.h
zephyr_compile_options("-Wno-missing-field-initializers")

# Disable error on checking non-literal format (zephyr)
# Reason: zephyr drivers/serial/uart_native_posix.c does not compile
zephyr_compile_options("-Wno-format-nonliteral")

# warn for any constants defined as 2.2 instead of 2.2f (as doubles)
# Unfortunately has to be disabled due to zephyr not compiling with it
zephyr_compile_options("-Wno-double-promotion")

Describe alternatives you've considered

Currently using the above code.

Additional context
None

@mkschreder mkschreder added the Enhancement Changes/Updates/Additions to existing features label May 29, 2023
@stephanosio stephanosio added area: Toolchains Toolchains area: Coding Guidelines Coding guidelines and style labels May 29, 2023
@stephanosio
Copy link
Member

stephanosio commented May 29, 2023

@stephanosio
Copy link
Member

No objection to this.

In fact, in the Toolchain WG, we have just recently discussed getting rid of the different "warning levels" in our build system and consolidating them into the single default warning list.

We just need someone to do the grunt work fixing all the existing violations before enabling these warnings.

@stephanosio
Copy link
Member

# treat all warnings as errors
zephyr_compile_options("-Werror")

It should be up to the user to decide whether they want to treat warnings as errors -- this is already configurable via CONFIG_COMPILER_WARNINGS_AS_ERRORS. Note that we build with this option enabled in the upstream CI.

# enable extra warnings
zephyr_compile_options("-Wextra")
# enable standard set of warnings (doesn't matter if already enabled)
zephyr_compile_options("-Wall")

We should refrain from using "all-in-one" warning flags like Wextra and Wall since their semantics (i.e. the warnings they enable) tend to vary among different compilers and even different versions of the same compiler. Instead, we should specify individual warnings that we want to enable.

@keith-zephyr
Copy link
Contributor

Note, the no-double-promotion was recently merged. #57154

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Coding Guidelines Coding guidelines and style area: Toolchains Toolchains Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

7 participants