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

Fix gnu folding constant #84341

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thughes
Copy link
Contributor

@thughes thughes commented Jan 21, 2025

When building with clang it warns:

    tests/kernel/common/src/pow2.c:30:6: error: variable length array folded
    to constant array as an extension [-Werror,-Wgnu-folding-constant]
    char static_array1[Z_POW2_CEIL(1)];
         ^

We want the variable length array folded into a constant array, so disable the warning.

Depends on #84063

Copy link
Collaborator

@npitre npitre left a comment

Choose a reason for hiding this comment

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

This is wrong.

Why would clang warn when the constant 1 is used? Why not with 2 or 3?
Why warn at all in the first place?

If this clang warning isn't useful, perhaps it should simply be disabled
altogether and not papered over each time it comes up. Or perhaps a clang
specific Z_POW2_CEIL() implementation is needed. But as presented this
makes no sense to me.

@thughes thughes force-pushed the fix-gnu-folding-constant branch from bb34034 to af79fa8 Compare February 7, 2025 18:49
Copy link
Collaborator

@npitre npitre left a comment

Choose a reason for hiding this comment

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

My previous concerns still stand.

Having to surround code with IN_WARNING_GNU_FOLDING_CONSTANT) and
TOOLCHAIN_ENABLE_CLANG_WARNING(TOOLCHAIN_WARNING_GNU_FOLDING_CONSTANT) is
rather unwieldy.

Question:

  • Why in the test is it necessary for only one line? Why not for the other
    lines that use the same construct?

  • Is this particular warning any useful in the first place?

I'm open to be convinced otherwise, but at the moment I think this
particular warning ought to be disabled globally for the entire tree.

@thughes thughes force-pushed the fix-gnu-folding-constant branch from af79fa8 to b24b7a8 Compare February 12, 2025 22:33
@thughes thughes marked this pull request as draft February 12, 2025 22:37
@thughes thughes force-pushed the fix-gnu-folding-constant branch 5 times, most recently from 0c4b316 to e3d47c5 Compare February 20, 2025 17:42
@thughes thughes force-pushed the fix-gnu-folding-constant branch 2 times, most recently from 3a7f097 to cf65f31 Compare March 10, 2025 22:15
When building with clang it warns:

tests/kernel/common/src/pow2.c:30:6: error: variable length array folded
to constant array as an extension [-Werror,-Wgnu-folding-constant]
char static_array1[Z_POW2_CEIL(1)];
     ^

We want the variable length array folded into a constant
array, so disable the warning.

Signed-off-by: Tom Hughes <tomhughes@chromium.org>
@thughes thughes force-pushed the fix-gnu-folding-constant branch from cf65f31 to 384f33c Compare April 3, 2025 21:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants