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

COMP: Suppress some Wformat-nonliteral warnings #4616

Merged
merged 1 commit into from
Apr 29, 2024
Merged

Conversation

seanm
Copy link
Contributor

@seanm seanm commented Apr 26, 2024

Introduced GCC_PRAGMA_PUSH macro and friends, similiar to existing CLANG_PRAGMA_PUSH and friends, but that applies to both GCC and Clang, which share many warning flags.

Added macros to suppess -Wformat-nonliteral warnings. Used them in a few places where the warnings are impossible to fix with ITK's current API.

Made -Wfloat-equal apply to GCC now, in addition to Clang.

Also added these macros as StatementMacros in the .clang-format file, so that clang-format formats them more nicely. Redid some formatting.

PR Checklist

  • No API changes were made (or the changes have been approved)
  • No major design changes were made (or the changes have been approved)
  • Added test (or behavior not changed)
  • Updated API documentation (or API not changed)
  • Added license to new files (if any)
  • Added Python wrapping to new files (if any) as described in ITK Software Guide Section 9.5
  • Added ITK examples for all new major features (if any)

Refer to the ITK Software Guide for
further development details if necessary.

@github-actions github-actions bot added type:Compiler Compiler support or related warnings type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Segmentation Issues affecting the Segmentation module labels Apr 26, 2024
@seanm
Copy link
Contributor Author

seanm commented Apr 26, 2024

This should fix some of these warnings:

https://open.cdash.org/viewBuildError.php?type=1&buildid=9572940

@dzenanz dzenanz requested a review from N-Dekker April 26, 2024 16:33
@hjmjohnson hjmjohnson force-pushed the suppress-warning branch 2 times, most recently from 566c36d to b3ef504 Compare April 27, 2024 12:00
@github-actions github-actions bot added the type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots label Apr 27, 2024
@hjmjohnson hjmjohnson added the action:ApplyClangFormat Add this label to a pull request to apply `clang-format` to the branch label Apr 27, 2024
@hjmjohnson
Copy link
Member

@seanm Ran clang-formatter:

./Utilities/Maintenance/clang-format.bash --clang-format /Users/johnsonhj/Dashboard/src/ITK/cmake-build-release/clang-format-Darwin --tracked

@hjmjohnson hjmjohnson force-pushed the suppress-warning branch 2 times, most recently from 6e75e99 to 47c9788 Compare April 28, 2024 12:45
@hjmjohnson hjmjohnson self-requested a review April 28, 2024 15:16
Copy link
Member

@hjmjohnson hjmjohnson left a comment

Choose a reason for hiding this comment

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

Thanks for the initial patches. I made some minor updates to get it to pass CI.

@seanm
Copy link
Contributor Author

seanm commented Apr 29, 2024

Thanks for the touch-ups!

Introduced ITK_GCC_PRAGMA_PUSH macro and friends, similiar
to existing CLANG_PRAGMA_PUSH and friends, but that applies
to both GCC and Clang, which share many warning flags.

Added macros to suppess -Wformat-nonliteral warnings. Used
them in a few places where the warnings are impossible to
fix with ITK's current API.

Made -Wfloat-equal apply to GCC now, in addition to Clang.

Also added these macros as StatementMacros in the .clang-format
file, so that clang-format formats them more nicely.
Redid some formatting.
@hjmjohnson hjmjohnson merged commit defa712 into master Apr 29, 2024
15 of 16 checks passed
@dzenanz dzenanz deleted the suppress-warning branch April 29, 2024 17:25
@SimonRit
Copy link

SimonRit commented May 2, 2024

This PR has removed CLANG_SUPPRESS_Wfloat_equal which was used in my code:
https://my.cdash.org/viewBuildError.php?buildid=2552633
There is a quick fix of course but other users might still use CLANG_SUPPRESS_Wfloat_equal. Was this intentional? I wonder why it's not been kept since CLANG_PRAGMA_PUSH and CLANG_PRAGMA_POP have been kept.

@seanm
Copy link
Contributor Author

seanm commented May 2, 2024

Hmm, although it's in a public header, I didn't imaging anyone using it.

I'll make a new MR to bring it back. Probably not until later today though, as I'm just recovering from a stomach bug.

@seanm
Copy link
Contributor Author

seanm commented May 4, 2024

@SimonRit #4644

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action:ApplyClangFormat Add this label to a pull request to apply `clang-format` to the branch area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Segmentation Issues affecting the Segmentation module type:Compiler Compiler support or related warnings type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants