Skip to content

Conversation

folkertdev
Copy link
Contributor

@folkertdev folkertdev commented Sep 3, 2025

Related to https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast (and very much modelled off of its implementation).

This lint is motivated by cleaning up the output of c2rust, which contains many .offset calls with by literal. The lint also nicely handles offsets by zero, which are unnecessary.

I'm aware that the feature freeze is still in place, but this was top-of-mind now. I'll patiently wait until the freeze is lifted.

changelog: [ptr_offset_by_literal]: add ptr_offset_by_literal lint

@rustbot
Copy link
Collaborator

rustbot commented Sep 3, 2025

r? @llogiq

rustbot has assigned @llogiq.
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 Sep 3, 2025
Copy link

github-actions bot commented Sep 3, 2025

Lintcheck changes for b0e37f3

Lint Added Removed Changed
clippy::ptr_offset_by_literal 41 0 0

This comment will be updated if you push new changes

@rustbot rustbot added S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work A-lint Area: New lints and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Sep 3, 2025
}

fn get_literal_bits<'tcx>(expr: &'tcx Expr<'tcx>) -> Option<u128> {
let ExprKind::Lit(lit) = expr.kind else {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a let-chain is a bit more concise

Suggested change
let ExprKind::Lit(lit) = expr.kind else {
if let ExprKind::Lit(lit) = expr.kind
&& let LitKind::Int(packed_u128, _) = lit.node
{
Some(packed_u128.get())
} else {
None
}

(the suggestion seems to be formatted weirdly, but it should (?) work correctly)

Method::Offset => "add",
Method::WrappingOffset => "wrapping_add",
},
Ordering::Equal => return Some(format!("{receiver}")),
Copy link
Contributor

Choose a reason for hiding this comment

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

This case looks a bit surprising, maybe add a short comment to it? Something like:

Suggested change
Ordering::Equal => return Some(format!("{receiver}")),
// `ptr.offset(0)` is equivalent to `ptr`, so no adjustment is needed
Ordering::Equal => return Some(format!("{receiver}")),

(why on earth do the suggestions look like this)

@rustbot

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Sep 14, 2025

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@folkertdev
Copy link
Contributor Author

CI fails on an invalid version attribute, I'm using #[clippy::version = "CURRENT_RUSTC_VERSION"] because it's unclear when this will be merged exactly. What is the intended approach here?

@samueltardieu
Copy link
Member

CI fails on an invalid version attribute, I'm using #[clippy::version = "CURRENT_RUSTC_VERSION"] because it's unclear when this will be merged exactly. What is the intended approach here?

1.92.0

It will be fixed if necessary.

@samueltardieu samueltardieu added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work labels Sep 17, 2025
@folkertdev
Copy link
Contributor Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants