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

Replace diagnostic pragmas with TOOLCHAIN_* macros #84065

Conversation

thughes
Copy link
Contributor

@thughes thughes commented Jan 16, 2025

The TOOLCHAIN_DISABLE_WARNING/TOOLCHAIN_ENABLE_WARNING macros are easier to read and compiler agnostic.

Depends on #84063

Fixes #84138

@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch 2 times, most recently from 226f7d9 to 55820c5 Compare January 16, 2025 18:00
@thughes thughes changed the title Replace diagnostic macros with toolchain disable macros Replace diagnostic pragmas with TOOLCHAIN_* macros Jan 16, 2025
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch from 55820c5 to 5e07390 Compare January 16, 2025 18:51
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch 4 times, most recently from ac4636e to 67c4390 Compare January 17, 2025 18:46
@thughes thughes marked this pull request as ready for review January 17, 2025 22:10
@cfriedt cfriedt assigned tejlmand and unassigned cfriedt Jan 18, 2025
@cfriedt
Copy link
Member

cfriedt commented Jan 18, 2025

signal.h looks fine.

I suspect @nordic-krch should also review

@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch 2 times, most recently from a143260 to d0fa2c4 Compare February 15, 2025 00:14
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch 4 times, most recently from 897df83 to baae26c Compare February 20, 2025 17:42
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch from baae26c to 9c7cca2 Compare March 10, 2025 22:12
thughes added 2 commits March 10, 2025 15:15
The TOOLCHAIN_IGNORE_WSHADOW_BEGIN and TOOLCHAIN_IGNORE_WSHADOW_END
macros can be replaced with the more generic
TOOLCHAIN_DISABLE_WARNING(TOOLCHAIN_WARNING_SHADOW) and
TOOLCHAIN_ENABLE_WARNING(TOOLCHAIN_WARNING_SHADOW) macros.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
The TOOLCHAIN_DISABLE_WARNING/TOOLCHAIN_ENABLE_WARNING macros are easier
to read and compiler agnostic.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch from 9c7cca2 to 9da9bf1 Compare March 10, 2025 22:15
@thughes thughes marked this pull request as draft March 11, 2025 22:57
Many warnings were disabled for all compilers, even though they are
gcc-specific warnings. Now that clang has -Wunknown-warning-option
enabled, this can cause compilation failures when building with clang
toolchains.

Use TOOLCHAIN_DISABLE_GCC_WARNING for all gcc-specific macros.

https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
https://clang.llvm.org/docs/DiagnosticsReference.html

Fixes: zephyrproject-rtos#84138

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
@thughes thughes force-pushed the replace-diagnostic-macros-with-toolchain-disable branch from 9da9bf1 to 8085f9a Compare March 14, 2025 22:40
@thughes thughes marked this pull request as ready for review March 14, 2025 23:53
@thughes thughes requested review from tejlmand and nordic-krch March 14, 2025 23:53
@thughes
Copy link
Contributor Author

thughes commented Mar 17, 2025

@tejlmand This one is ready now that all dependencies have been merged.

Copy link
Collaborator

@tejlmand tejlmand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for this and all the other cleanups 👍

@thughes
Copy link
Contributor Author

thughes commented Mar 20, 2025

I need another approver for this to be merged.

@dcpleung @cfriedt @nordic-krch Would any of you be able to take a look? Thanks!

@kartben kartben merged commit 1541174 into zephyrproject-rtos:main Mar 20, 2025
27 checks passed
@thughes thughes deleted the replace-diagnostic-macros-with-toolchain-disable branch March 21, 2025 16:47
thughes added a commit to thughes/zephyr that referenced this pull request Mar 21, 2025
These macros were recently introduced and replaced everywhere (zephyrproject-rtos#84065),
but this was missed.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
thughes added a commit to thughes/zephyr that referenced this pull request Mar 21, 2025
These macros were recently introduced and replaced everywhere (zephyrproject-rtos#84065),
but this was missed.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
thughes added a commit to thughes/zephyr that referenced this pull request Mar 21, 2025
…ning

zephyrproject-rtos#84065 replaced diagnostic pragmas with TOOLCHAIN_* macros, but we don't
need to use that here since __used is a cleaner way to indicate that the
function is used and will also prevent it from being optimized away at
link time if LTO is enabled.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
thughes added a commit to thughes/zephyr that referenced this pull request Mar 21, 2025
…ning

zephyrproject-rtos#84065 replaced diagnostic pragmas with TOOLCHAIN_* macros, but we don't
need to use that here since __used is a cleaner way to indicate that the
function is used and will also prevent it from being optimized away at
link time if LTO is enabled.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
kartben pushed a commit that referenced this pull request Mar 26, 2025
These macros were recently introduced and replaced everywhere (#84065),
but this was missed.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
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

Successfully merging this pull request may close these issues.

intel_adsp: xt-clang: unknown warning group -Wunknown-warning-option.
5 participants