Skip to content

[NFC] Silence some unused variable warnings #82042

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

Merged
merged 2 commits into from
Jun 12, 2025

Conversation

j-hui
Copy link
Contributor

@j-hui j-hui commented Jun 6, 2025

This patch addresses some warnings that come from variables that seem unused outside of recently-added assert() statements. Those warnings come from header files that are used by many translation units. This patch replaces them with ASSERT() to "use" the variables.

Clang complains that paramIndicesLength is unused. This block is already
guarded by CONDITIONAL_ASSERT_enabled(), so promoting assert() to
ASSERT() should not affect the behavior of assertions-disabled builds.
@j-hui j-hui requested review from jckarter and meg-gupta June 6, 2025 00:08
@j-hui
Copy link
Contributor Author

j-hui commented Jun 6, 2025

@swift-ci please smoke test

1 similar comment
@j-hui
Copy link
Contributor Author

j-hui commented Jun 7, 2025

@swift-ci please smoke test

@meg-gupta
Copy link
Contributor

These changes LGTM, thank you for fixing them!
Left a minor comment if you want to cherry pick this to 6.2.

Comments in PR swiftlang#80438, where this code comes from, already suggest using
ASSERT() instead of assert().
@j-hui j-hui force-pushed the silence-warnings branch from 4708c77 to 8aca536 Compare June 12, 2025 16:58
@j-hui
Copy link
Contributor Author

j-hui commented Jun 12, 2025

@swift-ci please smoke test

@j-hui j-hui enabled auto-merge June 12, 2025 16:59
@j-hui
Copy link
Contributor Author

j-hui commented Jun 12, 2025

Left a minor comment if you want to cherry pick this to 6.2.

@meg-gupta for super minor changes to assertions like this, do we need to cherry-pick it?

@meg-gupta
Copy link
Contributor

@j-hui I don't think it's necessary to cherry pick this.

@j-hui j-hui merged commit 270f56a into swiftlang:main Jun 12, 2025
3 checks passed
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.

2 participants