Skip to content

lint: enable clippy cast lints - #405

Draft
stringhandler wants to merge 1 commit into
BlockstreamResearch:masterfrom
stringhandler:st-cast-lints
Draft

lint: enable clippy cast lints#405
stringhandler wants to merge 1 commit into
BlockstreamResearch:masterfrom
stringhandler:st-cast-lints

Conversation

@stringhandler

Copy link
Copy Markdown
Contributor

cast_possible_wrap and cast_sign_loss have no hits at all. Turn them on.

cast_possible_truncation had four hits, all provably safe by construction; annotate them at the site instead of allowing the lint crate-wide, so the existing 'every cast should contain a code comment' convention is enforced rather than aspirational.

LLM suggested and generated

cast_possible_wrap and cast_sign_loss have no hits at all. Turn them on.

cast_possible_truncation had four hits, all provably safe by
construction; annotate them at the site instead of allowing the lint
crate-wide, so the existing 'every cast should contain a code comment'
convention is enforced rather than aspirational.
Comment thread codegen/src/main.rs
// Jets take a handful of parameters at most, so the index stays well
// inside `u8` (and inside the a-z range this names them from).
#[allow(clippy::cast_possible_truncation)]
let identifier = (b'a' + i as u8) as char;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In 724f851:

We should use u8::try_from and unwrap if we really believe this is impossible.

Comment thread src/num.rs
// A remainder modulo 256 is at most 255, so it fits in a `u8`.
#[allow(clippy::cast_possible_truncation)]
{
*byte = (value % 256) as u8;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In 724f851:

This is just confusing. We should write *byte = value as u8 and whitelist the lint with "truncation is deliberate".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I think let me fix these and then I'll undraft it

@stringhandler
stringhandler marked this pull request as draft September 1, 2026 11:57
@apoelstra

Copy link
Copy Markdown
Contributor

I think the ones in the division code are probably fine to leave as casts. But if you want to make them use try_from and expect as well, that's more correct (but results in noisier code and requires we trust the compiler to do the same analysis to eliminate the branches)

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