Skip to content

clippy: Raises result_large_err to squelch lint#6290

Closed
brooksprumo wants to merge 1 commit intoanza-xyz:masterfrom
brooksprumo:clippy/result_large_err
Closed

clippy: Raises result_large_err to squelch lint#6290
brooksprumo wants to merge 1 commit intoanza-xyz:masterfrom
brooksprumo:clippy/result_large_err

Conversation

@brooksprumo
Copy link
Copy Markdown

Problem

Upgrading Rust to 1.87.01 is blocked on a new clippy lint2, result_large_err3.

Summary of Changes

Raise the threshold for the lint to 265, since the largest size today is 264 bytes.

A subsequent PR can fix the underlying Error type to box the large variant, and remove the custom threshold.

By raising the threshold here, we unblock upgrading Rust, and we don't change any behavior.

Note that annotating the Error enums with #[allow(clippy::result_large_err)] does not squelch the lint unfortunately.

Fixes #6278

Footnotes

  1. https://github.com/anza-xyz/agave/issues/6279

  2. https://github.com/anza-xyz/agave/issues/6278

  3. https://rust-lang.github.io/rust-clippy/master/index.html#result_large_err

@brooksprumo brooksprumo marked this pull request as ready for review May 21, 2025 18:51
@brooksprumo brooksprumo requested review from steviez, t-nelson and yihau May 21, 2025 18:51
Copy link
Copy Markdown
Member

@yihau yihau left a comment

Choose a reason for hiding this comment

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

note: I've tried adding #[allow(clippy::result_large_err)] in several different places before. #5990

I don’t have a preference for any particular solution. just wanted to bring something up: we will allow all new large Result types under 265 bytes if we use this PR.

@brooksprumo
Copy link
Copy Markdown
Author

brooksprumo commented May 22, 2025

I implemented boxing the ErrorKind field inside solana_rpc_client_api::client_error::Error here: brooksprumo@36edeac, but it has two problems:

  1. solana_rpc_client_api::client_error::Error is public, so changing the type of kind is an API-breaking change. We cannot do this until v3.0.

  2. Even if we did change solana_rpc_client_api::client_error::Error, there are still other Result types larger than the default threshold: TpsClientResult is 144 bytes, and storage-bigtable/src/bigtable.rs Error is 176 bytes. So we'd have to change those as well, and changing them is also an API-breaking change.

So, for agave v2.x, I think the only option we have is to bump the threshold.

@brooksprumo
Copy link
Copy Markdown
Author

@steviez / @t-nelson, let me know if you'd like to review before I merge. I'll plan to merge tomorrow otherwise.

@t-nelson
Copy link
Copy Markdown

i'd prefer the explicit allows in offending call sites personally

@brooksprumo
Copy link
Copy Markdown
Author

i'd prefer the explicit allows in offending call sites personally

#5990 is where @yihau tried that previously. Can you take a look? It's a lot of annotations.

@brooksprumo
Copy link
Copy Markdown
Author

Closing. Now that we're on v3.0, we can make API-breaking changes. As such, a real fix (not changing the lint limit) is preferred.

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.

New result_large_err clippy lint blocks upgrading Rust to 1.87.0

3 participants