Fix -Wunused-command-line-argument warnings on Mac OS X#1967
Merged
Dead2 merged 1 commit intozlib-ng:developfrom Sep 27, 2025
Merged
Fix -Wunused-command-line-argument warnings on Mac OS X#1967Dead2 merged 1 commit intozlib-ng:developfrom
Dead2 merged 1 commit intozlib-ng:developfrom
Conversation
Contributor
WalkthroughAdds Apple-specific detection of -Werror=unused-command-line-argument and integrates it into ADDITIONAL_CHECK_FLAGS. Ensures subsequent compiler feature checks (notably -fno-semantic-interposition) are performed with these accumulated flags by temporarily setting and then resetting CMAKE_REQUIRED_FLAGS in CMake and propagating ADDITIONAL_CHECK_FLAGS in the configure script. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer
participant CMake as CMake
participant CC as Compiler
Dev->>CMake: Configure (APPLE)
CMake->>CC: Test -Werror=unused-command-line-argument
alt supported
CMake->>CMake: ADDITIONAL_CHECK_FLAGS += flag
else not supported
CMake->>CMake: No change
end
note over CMake: Prepare -fno-semantic-interposition check
CMake->>CMake: CMAKE_REQUIRED_FLAGS = ADDITIONAL_CHECK_FLAGS
CMake->>CC: Test -fno-semantic-interposition
CMake->>CMake: CMAKE_REQUIRED_FLAGS = ""
sequenceDiagram
autonumber
participant User as User
participant Conf as ./configure
participant CC as Compiler
User->>Conf: Run configure
Conf->>CC: Compile test with -Werror=unused-command-line-argument
alt supported
Conf->>Conf: ADDITIONAL_CHECK_FLAGS += flag
else not supported
Conf->>Conf: No change
end
note over Conf: Feature check uses accumulated flags
Conf->>CC: Compile test with $ADDITIONAL_CHECK_FLAGS + -fno-semantic-interposition
alt supported
Conf->>Conf: Record support
else not supported
Conf->>Conf: Record lack of support
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
configure (1)
683-683: Remove redundant write to test.cThe subsequent heredoc writes to $test.c; this line creates an unused test.c file.
-echo "" > test.c
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
CMakeLists.txt(2 hunks)configure(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-04-15T09:20:52.333Z
Learnt from: mtl1979
PR: zlib-ng/zlib-ng#1904
File: arch/riscv/Makefile.in:12-14
Timestamp: 2025-04-15T09:20:52.333Z
Learning: Optional feature flags in Makefile.in templates should default to empty strings. The configure script should detect feature support and update these variables accordingly during the configuration process.
Applied to files:
configure
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (79)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
- GitHub Check: macOS Clang ASAN (ARM64)
- GitHub Check: Windows GCC Compat No Opt
- GitHub Check: Windows ClangCl Win64
- GitHub Check: macOS Clang (Target 10.10)
- GitHub Check: Windows MSVC 2022 v140 Win64
- GitHub Check: EL9 GCC S390X DFLTCC ASAN
- GitHub Check: Ubuntu GCC AARCH64 Compat No Opt UBSAN
- GitHub Check: macOS GCC Symbol Prefix (ARM64)
🔇 Additional comments (4)
configure (2)
682-693: LGTM: detection of -Werror=unused-command-line-argument and propagation into ADDITIONAL_CHECK_FLAGSMatches the existing pattern for the other Apple/Clang warnings and will prevent false positives when probing -fno-semantic-interposition.
1014-1019: LGTM: include ADDITIONAL_CHECK_FLAGS when probing -fno-semantic-interpositionEnsures clang’s “unused-command-line-argument” becomes an error during the probe, avoiding incorrect enablement.
CMakeLists.txt (2)
408-411: LGTM: add Apple check for -Werror=unused-command-line-argumentProperly augments ADDITIONAL_CHECK_FLAGS alongside the existing Apple-specific warnings.
527-530: LGTM: scope CMAKE_REQUIRED_FLAGS around -fno-semantic-interposition checkUsing ADDITIONAL_CHECK_FLAGS here avoids false positives on Apple clang; flags are reset afterwards.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1967 +/- ##
===========================================
+ Coverage 81.60% 82.77% +1.17%
===========================================
Files 163 163
Lines 14073 14074 +1
Branches 3165 3166 +1
===========================================
+ Hits 11484 11650 +166
+ Misses 1597 1432 -165
Partials 992 992 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit