improve diagnostic for raw pointer field access with -> #139921

Kivooeo · 2025-04-16T13:39:46Z

This PR enhances the error messages emitted by the Rust compiler when users attempt to use the -> operator for field access on raw pointers or when dereferencing is needed. The changes aim to provide clearer guidance, by suggesting the correct use of the . operator and explicit dereferencing.

Before:

help: `xs` is a raw pointer; try dereferencing it
   |
LL |         (*xs)->count += 1;
   |         ++  +

Now:

help: use `.` on a dereferenced raw pointer instead
   |
LL -         xs->count += 1;
LL +         (*xs).count += 1;
   |

I added extra clarification in the message. Since this error occurs in the parser, we can't be certain that the type is a raw pointer. That's why the message includes only a small note in brackets. (In contrast, the message above is emitted in HIR, where we can check whether it's a raw pointer.)

Before:

  --> main.rs:11:11
   |
11 |         xs->count += 1;
   |           ^^
   |
   = help: the . operator will dereference the value if needed

After:

--> main.rs:11:11
   |
11 |         xs->count += 1;
   |           ^^
   |
   = help: the `.` operator will automatically dereference the value, except if the value is a raw pointer

rustbot · 2025-04-16T13:39:52Z

r? @estebank

rustbot has assigned @estebank.
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

fmease · 2025-04-16T14:08:49Z

compiler/rustc_hir_typeck/src/expr.rs

-            Applicability::MaybeIncorrect,
+        let full_span = base.span.to(field.span);
+
+        let base_snippet = self


Instead of using span_to_snippet (best to be avoided if possible) just to use it verbatim in the suggestion, you can use a multi-part suggestion (multipart_suggestion_verbose).

The argument to this method should probably look something like

vec![ (base.span.shrink_to_lo(), "(*".into()), (base.span.to(field.span.shrink_to_lo()), format!(").{}", field.name)), ]

I haven't verified these spans, you might need to play around a bit.

You could actually make this a three-part suggestion, not just a two-part one.

Yeah, I tried this thing as one of first my attemps to make it work

let suggestion = vec![ (base.span.shrink_to_lo(), "(*".to_string()), (base.span.shrink_to_hi(), format!(").{}", field.name)), ];

But this didn't work for me because it gives
(*xs).count->count += 1;
So I gave up and went this way you can see in PR, I surely will play around it a little more.
But I have also new question, how do i generate new stderr files? Because CI failing on comparing them because I didn't change them

cc https://rustc-dev-guide.rust-lang.org/

compiler-errors · 2025-04-16T17:01:07Z

Please squash the last commit into the first

mejrs · 2025-04-16T20:25:29Z

compiler/rustc_parse/messages.ftl

@@ -248,7 +248,7 @@ parse_expected_trait_in_trait_impl_found_type = expected a trait, found type

 parse_expr_rarrow_call = `->` used for field access or method call
    .suggestion = try using `.` instead
-    .help = the `.` operator will dereference the value if needed
+    .help = the `.` operator will dereference the value if needed (if value is raw pointer you have to derefernce it by yourself)


Suggested change

.help = the `.` operator will dereference the value if needed (if value is raw pointer you have to derefernce it by yourself)

.help = the `.` operator will dereference the value if needed (if value is a raw pointer you have to dereference it by yourself)

Anyhow, I'm not a fan of this phrasing. I'd be inclined to say something like "field access expressions/method call syntax can perform autodereferencing". Thoughts?

Oh, it's actually interesting because i fixed this typo in one of my squashed commit, seems it was lost somehow. But anyway. Do you prefer something like this?
.help = field access and method calls automatically dereference values (but raw pointers must be dereferenced manually)
Is that makes more sense?

I've thought about it some more and I'm leaning towards:

the `.` operator will automatically dereference the operand, except if the operand is a raw pointer

Yeah, this works too, but the only thing I’m not really sure about is the word operand. I feel like value is more commonly used, what do you think about it?

The reference uses "operand" quite a lot, but I don't see it used often.

Fixed, I'm going to stick with value variant since it sounds more natural

compiler/rustc_hir_typeck/src/expr.rs

tests/ui/parser/issues/issue-118530-ice.stderr

WaffleLapkin · 2025-04-20T14:30:25Z

tests/ui/parser/issues/issue-118530-ice.stderr

-   = help: the `.` operator will dereference the value if needed
-help: try using `.` instead
-   |
-LL -     attr::fn bar() -> String {
-LL +     attr::fn bar() . String {
-   |


I don't like the removal of these suggestions, this seems like a big step backwards.

I get the problem you are having with the diagnostics clashing, but I think we simply have to accept that. There isn't a good solution available at the moment. So you test will just have to be not-//@ run-rustfix. (the users can choose which suggestion to apply, so it's fine-ish to provide clashing suggestions)

The proper solution might look something like having a ast node<->diagnostics map, instead of emitting diagnostics in parser stash them there, then in the typechecker override the diagnostic if it was stashed. But we simply don't have the infrastructure to do this (and it'd be hard to add).

The suggestion in issue-118530-ice to replace -> with . feels off to me at first glance. Without this suggestions the error looks cleaner, but may be wrong.

I removed the rustfix in expr-rarrow-call.rs for two reasons:

There was a conflict between replacing -> with . and -> with (*var)., which caused the tests to fail.

Since the HIR provides more context than the parser, I opted to handle (*var). replacements there and removed it from the parser, keeping only a detailed error message with no suggestions in parser.

I considered creating separate errors for expr-call-rarrow and expr-call-rarrow-raw, but ran into issues:

Why not handle both in HIR? The parser replaces -> with ., so HIR can't tell if it was originally an arrow. This forces HIR to emit an error for a non-existent field.

Why not handle both in the parser? The parser lacks type information, making it impossible to distinguish between raw pointers and references.

There is no conflict in expr-rarrow-call.rs, I only see the parser diagnostic triggering there.

Since the HIR provides more context than the parser, I opted to handle (*var). replacements there and removed it from the parser, keeping only a detailed error message with no suggestions in parser.

As I already said, I don't agree with this decision and am asking you to revert this. The typecheck diagnostic is only triggered when the -> to . parser recovery triggers a type errors (like in the case of lhs being a pointer). So in most cases parser suggestion is valuable.

The conflict between diagnostics is annoying, but it is unavoidable with the current infra.

WaffleLapkin · 2025-04-20T14:34:20Z

@Kivooeo as errs said, a few days of inactivity is very common, reviewing PRs takes time and energy and people have lifes besides rustc :)

The sponsor page talks more about being on review rotation, doing PR review as a big part of the work. I still occasionally review PRs if I feel like it. I wouldn't assign myself to the PR if I wouldn't want to review it.

Kivooeo · 2025-04-20T22:36:31Z

@WaffleLapkin check this changes, tldr I reverted sugesstion attribute for Rarrow span in parser, removed rustfix in my testfile, updated error messages. I actually little bit ruined commits, starting this in master was awful idea, I will try to fix it tomorrow if could, if no I will close this and open new PR in separate branch
upd: fixed branch

WaffleLapkin

Looks good to me, modulo 1 nit

WaffleLapkin · 2025-04-21T17:41:13Z

tests/ui/parser/expr-rarrow-call.rs

@@ -1,4 +1,3 @@
-//@ run-rustfix


This can return to being run-rustfix

true, fixed

WaffleLapkin

Thanks!

WaffleLapkin · 2025-04-22T13:04:06Z

bors · 2025-04-22T13:04:09Z

📌 Commit b5e8f1f has been approved by WaffleLapkin

It is now in the queue for this repository.

…enton Rollup of 8 pull requests Successful merges: - rust-lang#139617 (Use posix_spawn on cygwin) - rust-lang#139921 (improve diagnostic for raw pointer field access with ->) - rust-lang#140031 (compiletest: Fix deadline bugs in new executor) - rust-lang#140072 (handle function alignment in miri) - rust-lang#140104 (Fix auto diff failing on inherent impl blocks) - rust-lang#140124 (Update books) - rust-lang#140144 (Handle another negated literal in `eat_token_lit`.) - rust-lang#140149 (test_nan: ensure the NAN contant is quiet) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#139921 - Kivooeo:master, r=WaffleLapkin improve diagnostic for raw pointer field access with -> This PR enhances the error messages emitted by the Rust compiler when users attempt to use the `->` operator for field access on raw pointers or when dereferencing is needed. The changes aim to provide clearer guidance, by suggesting the correct use of the `.` operator and explicit dereferencing. **Before:** ``` help: `xs` is a raw pointer; try dereferencing it | LL | (*xs)->count += 1; | ++ + ``` **Now:** ``` help: use `.` on a dereferenced raw pointer instead | LL - xs->count += 1; LL + (*xs).count += 1; | ``` I added extra clarification in the message. Since this error occurs in the parser, we can't be certain that the type is a raw pointer. That's why the message includes only a small note in brackets. (In contrast, the message above is emitted in HIR, where we *can* check whether it's a raw pointer.) **Before:** ``` --> main.rs:11:11 | 11 | xs->count += 1; | ^^ | = help: the . operator will dereference the value if needed ``` **After:** ``` --> main.rs:11:11 | 11 | xs->count += 1; | ^^ | = help: the `.` operator will automatically dereference the value, except if the value is a raw pointer ```

rustbot assigned estebank Apr 16, 2025

rustbot added S-waiting-on-review T-compiler labels Apr 16, 2025

fmease reviewed Apr 16, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits S-waiting-on-author labels Apr 16, 2025

This comment has been minimized.

Sign in to view

Kivooeo force-pushed the master branch from 168720a to a63e3d7 Compare April 16, 2025 17:41

rustbot removed has-merge-commits S-waiting-on-author labels Apr 16, 2025

Kivooeo force-pushed the master branch from a63e3d7 to d98bd39 Compare April 16, 2025 17:47

mejrs reviewed Apr 16, 2025

View reviewed changes

Kivooeo force-pushed the master branch from d98bd39 to 2ce0206 Compare April 16, 2025 21:34

This comment has been minimized.

Sign in to view

rustbot added has-merge-commits S-waiting-on-author labels Apr 16, 2025

Kivooeo force-pushed the master branch from a84a6cc to e06520a Compare April 16, 2025 21:54

rustbot removed has-merge-commits S-waiting-on-author labels Apr 16, 2025

Kivooeo force-pushed the master branch 2 times, most recently from 13d0ac8 to 11a6b40 Compare April 16, 2025 22:02

Urgau mentioned this pull request Apr 17, 2025

Fix new-line at the start in no_merges handler rust-lang/triagebot#1941

Merged

WaffleLapkin reviewed Apr 17, 2025

View reviewed changes

compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved

WaffleLapkin reviewed Apr 17, 2025

View reviewed changes

compiler/rustc_hir_typeck/src/expr.rs Outdated Show resolved Hide resolved

Kivooeo force-pushed the master branch 3 times, most recently from 340d7ee to 9ee67a5 Compare April 17, 2025 19:16

WaffleLapkin requested changes Apr 20, 2025

View reviewed changes

rustbot added S-waiting-on-author and removed S-waiting-on-review labels Apr 20, 2025

This comment has been minimized.

Sign in to view

rustbot added the has-merge-commits label Apr 20, 2025

Kivooeo force-pushed the master branch from fdd35a2 to 02d3653 Compare April 21, 2025 09:53

rustbot removed has-merge-commits S-waiting-on-author labels Apr 21, 2025

WaffleLapkin reviewed Apr 21, 2025

View reviewed changes

Kivooeo force-pushed the master branch 2 times, most recently from acab75e to d9d8a9c Compare April 21, 2025 17:47

This comment has been minimized.

Sign in to view

Kivooeo force-pushed the master branch from e22ce51 to 9788324 Compare April 21, 2025 18:23

This comment has been minimized.

Sign in to view

Kivooeo force-pushed the master branch from 090d1c6 to eed675d Compare April 21, 2025 18:58

This comment has been minimized.

Sign in to view

improve diagnostic for raw pointer field access using ->

b5e8f1f

Kivooeo force-pushed the master branch from 0f82d47 to b5e8f1f Compare April 21, 2025 19:53

WaffleLapkin approved these changes Apr 22, 2025

View reviewed changes

bors added the S-waiting-on-bors label Apr 22, 2025

ChrisDenton mentioned this pull request Apr 22, 2025

Rollup of 8 pull requests #140165

Merged

bors merged commit 8089e31 into rust-lang:master Apr 23, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve diagnostic for raw pointer field access with -> #139921

improve diagnostic for raw pointer field access with -> #139921

Kivooeo commented Apr 16, 2025 •

edited

Loading

rustbot commented Apr 16, 2025

fmease Apr 16, 2025 •

edited

Loading

Kivooeo Apr 16, 2025 •

edited

Loading

fmease Apr 16, 2025

fmease Apr 16, 2025

This comment has been minimized.

compiler-errors commented Apr 16, 2025

This comment has been minimized.

mejrs Apr 16, 2025

mejrs Apr 16, 2025

Kivooeo Apr 16, 2025 •

edited

Loading

mejrs Apr 16, 2025 •

edited

Loading

Kivooeo Apr 16, 2025

mejrs Apr 16, 2025 •

edited

Loading

Kivooeo Apr 16, 2025

This comment has been minimized.

WaffleLapkin Apr 20, 2025

Kivooeo Apr 20, 2025

WaffleLapkin Apr 20, 2025

WaffleLapkin commented Apr 20, 2025

This comment has been minimized.

Kivooeo commented Apr 20, 2025 •

edited

Loading

WaffleLapkin left a comment

WaffleLapkin Apr 21, 2025

Kivooeo Apr 21, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

WaffleLapkin left a comment

WaffleLapkin commented Apr 22, 2025

bors commented Apr 22, 2025

	.help = the `.` operator will dereference the value if needed (if value is raw pointer you have to derefernce it by yourself)
	.help = the `.` operator will dereference the value if needed (if value is a raw pointer you have to dereference it by yourself)

improve diagnostic for raw pointer field access with -> #139921

improve diagnostic for raw pointer field access with -> #139921

Conversation

Kivooeo commented Apr 16, 2025 • edited Loading

rustbot commented Apr 16, 2025

fmease Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Kivooeo Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

compiler-errors commented Apr 16, 2025

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kivooeo Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

mejrs Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mejrs Apr 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WaffleLapkin commented Apr 20, 2025

This comment has been minimized.

Kivooeo commented Apr 20, 2025 • edited Loading

WaffleLapkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

WaffleLapkin left a comment

Choose a reason for hiding this comment

WaffleLapkin commented Apr 22, 2025

bors commented Apr 22, 2025

Kivooeo commented Apr 16, 2025 •

edited

Loading

fmease Apr 16, 2025 •

edited

Loading

Kivooeo Apr 16, 2025 •

edited

Loading

Kivooeo Apr 16, 2025 •

edited

Loading

mejrs Apr 16, 2025 •

edited

Loading

mejrs Apr 16, 2025 •

edited

Loading

Kivooeo commented Apr 20, 2025 •

edited

Loading