Skip to content
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

Add help for trying to do C-like pointer arithmetics #112261

Merged
merged 1 commit into from
Jun 12, 2023

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Jun 4, 2023

This PR adds help messages for these cases:

fn main() {
    let ptr1: *const u32 = std::ptr::null();
    let ptr2: *const u32 = std::ptr::null();
    let a = ptr1 + 5;
    let b = ptr1 - 5;
    let c = ptr2 - ptr1;
    let d = ptr1[5];
}

Current Output

error[E0369]: cannot add `{integer}` to `*const u32`
 --> tests/ui/typeck/issue-112252-ptr-arithmetics-help.rs:4:18
  |
4 |     let a = ptr1 + 5; //~ ERROR cannot add
  |             ---- ^ - {integer}
  |             |
  |             *const u32

error[E0369]: cannot subtract `{integer}` from `*const u32`
 --> tests/ui/typeck/issue-112252-ptr-arithmetics-help.rs:5:18
  |
5 |     let b = ptr1 - 5; //~ ERROR cannot subtract
  |             ---- ^ - {integer}
  |             |
  |             *const u32

error[E0369]: cannot subtract `*const u32` from `*const u32`
 --> tests/ui/typeck/issue-112252-ptr-arithmetics-help.rs:6:18
  |
6 |     let c = ptr2 - ptr1; //~ ERROR cannot subtract
  |             ---- ^ ---- *const u32
  |             |
  |             *const u32

error[E0608]: cannot index into a value of type `*const u32`
 --> tests/ui/typeck/issue-112252-ptr-arithmetics-help.rs:7:13
  |
7 |     let d = ptr1[5]; //~ ERROR cannot index
  |             ^^^^^^^

error: aborting due to 4 previous errors

Output After This PR

error[E0369]: cannot add `{integer}` to `*const u32`
  --> $DIR/issue-112252-ptr-arithmetics-help.rs:6:20
   |
LL |     let _a = _ptr1 + 5;
   |              ------^--
   |              |       |
   |              |       {integer}
   |              *const u32
   |              help: consider using `wrapping_add` or `add` for pointer + {integer}: `_ptr1.wrapping_add(5)`

error[E0369]: cannot subtract `{integer}` from `*const u32`
  --> $DIR/issue-112252-ptr-arithmetics-help.rs:7:20
   |
LL |     let _b = _ptr1 - 5;
   |              ------^--
   |              |       |
   |              |       {integer}
   |              *const u32
   |              help: consider using `offset` for pointer - {integer}: `unsafe { _ptr1.offset(-5) }`

error[E0369]: cannot subtract `*const u32` from `*const u32`
  --> $DIR/issue-112252-ptr-arithmetics-help.rs:8:20
   |
LL |     let _c = _ptr2 - _ptr1;
   |              ------^------
   |              |       |
   |              |       *const u32
   |              *const u32
   |              help: consider using `offset_from` for pointer - pointer if the pointers point to the same allocation: `_ptr2.offset_from(_ptr1)`

error[E0608]: cannot index into a value of type `*const u32`
  --> $DIR/issue-112252-ptr-arithmetics-help.rs:9:14
   |
LL |     let _d = _ptr1[5];
   |              ^^^^^^^^
   |
help: consider using `wrapping_add` or `add` for indexing into raw pointer
   |
LL |     let _d = _ptr1.wrapping_add(5);
   |              ~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 4 previous errors

Closes #112252.

@rustbot
Copy link
Collaborator

rustbot commented Jun 4, 2023

r? @eholk

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 4, 2023
@@ -2870,6 +2870,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
);
}
}

if let ty::RawPtr(..) = base_t.kind() && idx_t.is_integral() {
err.span_help(
Copy link
Member

Choose a reason for hiding this comment

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

Can this be a span_suggestion

Copy link
Member Author

@jieyouxu jieyouxu Jun 4, 2023

Choose a reason for hiding this comment

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

I was hesistant to make them suggestions because ptr::add and friends are unsafe functions.

If I were to make it into a suggestion, it would become something like

error[E0608]: cannot index into a value of type `*const u32`
 --> tests/ui/typeck/issue-112252-ptr-arithmetics-help.rs:9:13
  |
9 |     let d = ptr1[5]; //~ ERROR cannot index
  |             ^^^^^^^ help: consider using `add` for indexing into raw pointer: `unsafe { ptr1.add(5) }`

Copy link
Member

Choose a reason for hiding this comment

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

wrapping_add isn't unsafe. Although C pointer arithmetic does have the same preconditions as add I think. But I don't think suggestions should include unsafe code. add should still be mentioned in the help though.

Copy link
Member Author

@jieyouxu jieyouxu Jun 4, 2023

Choose a reason for hiding this comment

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

wrapping_add isn't unsafe.

Good catch, I'll refer to that instead of add.

But I don't think suggestions should include unsafe code.

I think I agree with this. I can undo the change to make them suggestions and revert them to being help notes if this is deemed more suitable.

Copy link
Member

@WaffleLapkin WaffleLapkin Jun 8, 2023

Choose a reason for hiding this comment

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

You can have both a suggestion and a note although having add in the suggestion text is also fine

if op.node == hir::BinOpKind::Add {
match (lhs_ty.kind(), rhs_ty) {
(RawPtr(..), rhs_ty) if rhs_ty.is_integral() => {
err.span_help(
Copy link
Member

Choose a reason for hiding this comment

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

similarly, and all other usages of span_help

@jieyouxu jieyouxu force-pushed the c-like-ptr-arithmetics-diagnostics branch 2 times, most recently from 50e4123 to d09f33d Compare June 4, 2023 08:22
@jieyouxu jieyouxu force-pushed the c-like-ptr-arithmetics-diagnostics branch from d09f33d to 7ab91e5 Compare June 4, 2023 09:34
err.span_suggestion(
expr.span,
"consider using `offset_from` for pointer - pointer",
format!("unsafe {{ {lhs_snip}.offset_from({rhs_snip}) }}"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a safe suggestion we could make here? I didn't find one immediately after poking around the docs a little...

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find one either...

Copy link
Member

Choose a reason for hiding this comment

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

you should at least add "if the pointers point to the same allocation"

@jieyouxu jieyouxu force-pushed the c-like-ptr-arithmetics-diagnostics branch from 7ab91e5 to 4ae70a7 Compare June 6, 2023 07:19
compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/op.rs Outdated Show resolved Hide resolved
compiler/rustc_hir_typeck/src/op.rs Outdated Show resolved Hide resolved
@jieyouxu jieyouxu force-pushed the c-like-ptr-arithmetics-diagnostics branch from 4ae70a7 to 63d643d Compare June 9, 2023 02:01
@WaffleLapkin
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 12, 2023

📌 Commit 63d643d has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2023
@bors
Copy link
Contributor

bors commented Jun 12, 2023

⌛ Testing commit 63d643d with merge fd0a331...

@bors
Copy link
Contributor

bors commented Jun 12, 2023

☀️ Test successful - checks-actions
Approved by: WaffleLapkin
Pushing fd0a331 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 12, 2023
@bors bors merged commit fd0a331 into rust-lang:master Jun 12, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 12, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (fd0a331): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.3%, 0.5%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.1% [-1.1%, -1.1%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.5% [-1.5%, -1.5%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.5% [-1.5%, -1.5%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 647.92s -> 648.279s (0.06%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Diagnostics when trying to do pointer arithmetic like in C
9 participants