Skip to content

Fix redundant closure warnings in Rust 1.82 #4125

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

drganjoo
Copy link
Contributor

@drganjoo drganjoo commented May 9, 2025

Problem

In Rust 1.82, the #[allow(clippy::redundant_closure)] attribute is not functioning as intended, causing warnings that are elevated to errors in our build process. Specifically, when generating code in the form:

input.map(#[allow(clippy::redundant_closure)] |v| MaybeConstrained::Constrained(v))

Clippy still reports a redundant closure error despite the allow attribute.

Solution

This PR implements a temporary workaround until the Clippy issue is fixed. Instead of relying on the allow attribute, we now generate code that follows Clippy's recommendation by using function references directly:

input.map(MaybeConstrained::Constrained)

For collection types like Vec and HashMap, we're using .unwrap_or_default() instead of closure-wrapped constructors.

Implementation Details

  • Added special handling for empty collections
  • Modified code generation to produce different output based on context
  • Preserved existing behavior for cases where we need to use closures (e.g., with Box::new, .into())

Testing

  • Verified code generation produces the expected output
  • Confirmed build succeeds without warnings when using the new code generator

The workaround will be revisited once the Clippy issue is resolved in a future Rust version.

@drganjoo drganjoo requested review from a team as code owners May 9, 2025 18:30
Copy link

github-actions bot commented May 9, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/clippy-warn branch from 18c82b5 to 4597059 Compare May 12, 2025 13:31
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@drganjoo drganjoo marked this pull request as draft May 13, 2025 11:58
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

@drganjoo drganjoo force-pushed the fahadzub/clippy-warn branch from 22a3c5d to 045dcb1 Compare May 22, 2025 11:33
Copy link

A new generated diff is ready to view.

A new doc preview is ready to view.

landonxjames added a commit that referenced this pull request Jun 3, 2025
Note we expect some failures here, at least until the following are merged:

#4125

#4146
Copy link

github-actions bot commented Jun 3, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

Copy link

github-actions bot commented Jun 4, 2025

A new generated diff is ready to view.

A new doc preview is ready to view.

@landonxjames
Copy link
Contributor

Note that in #4154 I added a -A clippy::redundant_closure to our RUSTFLAGS to get around this lint. Seems like it caused a merge conflict here, but should be easy to fix and disable for your own testing.

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.

3 participants