Skip to content

cast_possible_truncation: Support size_of, align_of, and size_of_val#15488

Open
RunDevelopment wants to merge 4 commits into
rust-lang:masterfrom
RunDevelopment:const-eval-size_of
Open

cast_possible_truncation: Support size_of, align_of, and size_of_val#15488
RunDevelopment wants to merge 4 commits into
rust-lang:masterfrom
RunDevelopment:const-eval-size_of

Conversation

@RunDevelopment

@RunDevelopment RunDevelopment commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

fixes parts of #9613, related to #12962

Unlike #12962, I added support for size_of, align_of, and size_of_val to ConstEval, so all lints using it benefit from this change.

cast_possible_truncation itself changed relatively little. Functions calls are now delegated to const eval and ptr-sized to fixed-sized casts now consider from_nbits.

changelog: [cast_possible_truncation]: support size_of, align_of, and size_of_val


My intention with this PR is not to superceed #12962, but to get in a simple improvement while I work on #15342.

@rustbot

rustbot commented Aug 14, 2025

Copy link
Copy Markdown
Collaborator

r? @blyxyas

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Aug 14, 2025
@github-actions

github-actions Bot commented Aug 14, 2025

Copy link
Copy Markdown

Lintcheck changes for c5a67a8

Lint Added Removed Changed
clippy::arithmetic_side_effects 0 46 5
clippy::assertions_on_constants 1 0 0
clippy::cast_possible_truncation 0 13 2
clippy::eq_op 7 0 0
clippy::identity_op 3 0 0
clippy::if_same_then_else 1 0 0

This comment will be updated if you push new changes

@rustbot

rustbot commented Sep 4, 2025

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (possibly e9b7045) made this pull request unmergeable. Please resolve the merge conflicts.

@blyxyas blyxyas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great contribution! Sorry for the late review, I've been very busy this past two weeks.

View changes since this review

Comment on lines +518 to +522
ExprKind::Call(callee, [_]) => match self.get_fn_diagnostic_name(callee) {
// `align_of_val` doesn't have a diagnostic name, unfortunately.
// Some(sym::mem_align_of_val) => self.align_of_call(callee),
Some(sym::mem_size_of_val) => self.size_of_call(callee),
_ => None,

@blyxyas blyxyas Sep 5, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the other comment

Suggested change
ExprKind::Call(callee, [_]) => match self.get_fn_diagnostic_name(callee) {
// `align_of_val` doesn't have a diagnostic name, unfortunately.
// Some(sym::mem_align_of_val) => self.align_of_call(callee),
Some(sym::mem_size_of_val) => self.size_of_call(callee),
_ => None,

Comment on lines +508 to +517
ExprKind::Call(callee, []) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) => self.size_of_call(callee),
_ => None,
},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[..] The rest pattern also allows for zero-sized arrays, no need for the extra branch.

Suggested change
ExprKind::Call(callee, []) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) => self.size_of_call(callee),
_ => None,
},
ExprKind::Call(callee, [..]) => match self.get_fn_diagnostic_name(callee) {
Some(sym::i8_legacy_fn_max_value) => Some(Constant::Int(i8::MAX as u128)),
Some(sym::i16_legacy_fn_max_value) => Some(Constant::Int(i16::MAX as u128)),
Some(sym::i32_legacy_fn_max_value) => Some(Constant::Int(i32::MAX as u128)),
Some(sym::i64_legacy_fn_max_value) => Some(Constant::Int(i64::MAX as u128)),
Some(sym::i128_legacy_fn_max_value) => Some(Constant::Int(i128::MAX as u128)),
Some(sym::mem_align_of) => self.align_of_call(callee),
Some(sym::mem_size_of) | Some(sym::mem_size_of_val)=> self.size_of_call(callee),
_ => None,
},

}
}

fn align_of_call(&self, callee: &Expr<'_>) -> Option<Constant<'tcx>> {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add some documentation here for these two functions?

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 5, 2025
@blyxyas

blyxyas commented Sep 23, 2025

Copy link
Copy Markdown
Member

ping @RunDevelopment 🏓 I'm not sure if you saw my review or if it just flew under the radar?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants