Add help message about function pointers #105552

mattjperez · 2022-12-11T07:56:21Z

rustbot · 2022-12-11T07:56:27Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information.

mattjperez · 2022-12-11T07:57:21Z

r? rust-lang/diagnostics

mattjperez · 2022-12-11T08:23:37Z

Ah, i forgot to run the test suite 😅.
Will fix it and ping the reviewer when it's ready.

TaKO8Ki · 2022-12-11T13:43:25Z

@rustbot author

When you are ready for the review, please use @rustbot review :)

compiler/rustc_hir_typeck/src/coercion.rs

compiler/rustc_infer/src/infer/error_reporting/suggest.rs

mattjperez · 2022-12-19T22:00:04Z

@rustbot ready

src/test/ui/fn/signature-error-reporting-under-verbose.stderr

src/test/ui/higher-rank-trait-bounds/hrtb-exists-forall-fn.stderr

compiler/rustc_infer/src/infer/error_reporting/suggest.rs

compiler-errors · 2023-01-09T20:46:34Z

r? @compiler-errors I'll review this soon

compiler-errors · 2023-01-11T00:11:05Z

src/test/ui/fn/fn-item-type.rs

Where did these notes go? Is there a good reason they disappeared?

The note about fn items being unique is still there, however the rest of the deleted notes are making a suggestion that would not result in working code.

If you follow the advice of casting to function pointers (in this example), this is the result

error[E0308]: arguments to this function are incorrect --> cases/1.rs:19:5 | 19 | eq(foo::<u8> as fn(isize) -> isize, bar::<u8> as fn(isize) -> isize); | ^^ | note: expected `*const _`, found fn pointer --> cases/1.rs:19:8 | 19 | eq(foo::<u8> as fn(isize) -> isize, bar::<u8> as fn(isize) -> isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: expected raw pointer `*const _` found fn pointer `fn(isize) -> isize` note: expected `*const _`, found fn pointer --> cases/1.rs:19:41 | 19 | eq(foo::<u8> as fn(isize) -> isize, bar::<u8> as fn(isize) -> isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: expected raw pointer `*const _` found fn pointer `fn(isize) -> isize` note: function defined here --> /home/mperez/projects/rustlang/library/core/src/ptr/mod.rs:1827:8 | 1827 | pub fn eq<T: ?Sized>(a: *const T, b: *const T) -> bool { | ^^ error: aborting due to previous error For more information about this error, try `rustc --explain E0308`.

So while the note might be useful for some cases, it wouldn't be a good suggestion in this case.

I'll come up with a couple other similar cases real quick to see if it might be a good/useful suggestion there.

Sorry, I'm still having a hard time understanding why this suggestion is incorrect for this UI test. Why are you pointing at std::ptr::eq when the UI test defines its own fn eq<T>(x: T, y: T) {}?

For the record, this code works:

fn eq<T: Eq>(a: T, b: T) {} fn a() {} fn b() {} fn main() { eq(a as fn(), b as fn()); // ^^^^^^^ ^^^^^^^ }

You were right. I mistakenly didn't include the locally defined fn eq when verifying the output.

So can we bring this suggestion back then?

mattjperez · 2023-01-21T20:53:15Z

@rustbot ready

compiler-errors

I'm still pretty sad that we're not even vaguely suggesting to use as to turn a FnDef into a FnPtr. We should be nudging the user to do something even if it's not always a structured suggestion they can apply.

tests/ui/reify-intrinsic.stderr

compiler-errors · 2023-01-22T17:22:43Z

src/test/ui/fn/fn-item-type.rs

Sorry, I'm still having a hard time understanding why this suggestion is incorrect for this UI test. Why are you pointing at std::ptr::eq when the UI test defines its own fn eq<T>(x: T, y: T) {}?

For the record, this code works:

fn eq<T: Eq>(a: T, b: T) {} fn a() {} fn b() {} fn main() { eq(a as fn(), b as fn()); // ^^^^^^^ ^^^^^^^ }

compiler/rustc_infer/src/infer/error_reporting/suggest.rs

mattjperez · 2023-01-24T16:01:07Z

@compiler-errors Please advise on

~~tests/ui/reify-intrinsic.stderr:8 : suggestion doesn't work~~
tests/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.thir.stderr: works without #[target_feature(enable = "sse2")]
tests/ui/rfcs/rfc-2396-target_feature-11/fn-ptr.mir.stderr: works without #[target_feature(enable = "sse2")]

tests/ui/issues/issue-59488.stderr

tests/ui/reify-intrinsic.stderr

- On compiler-error's suggestion of moving this lower down the stack, along the path of `report_mismatched_types()`, which is used by `rustc_hir_analysis` and `rustc_hir_typeck`. - update ui tests, add test - add suggestions for references to fn pointers - modify `TypeErrCtxt::same_type_modulo_infer` to take `T: relate::Relate` instead of `Ty`

compiler-errors

This is fine for now, though I left a few follow-up notes I'd like to see in a future PR, perhaps.

compiler-errors · 2023-01-24T22:36:19Z

tests/ui/fn/fn-item-type.stderr

              found fn item `fn(_) -> _ {bar::<u8>}`
-   = note: different `fn` items always have unique types, even if their signatures are the same
-   = help: change the expected type to be function pointer `fn(isize) -> isize`
-   = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `foo::<u8> as fn(isize) -> isize`


When we have two FnDefs, we should mention that the user can use an as to turn them into pointers.

compiler-errors · 2023-01-24T22:36:57Z

tests/ui/fn/fn-item-type.stderr

   = note: expected fn item `fn(_) -> _ {foo::<u8>}`
           found fn pointer `fn(_) -> _`
-   = help: change the expected type to be function pointer `fn(isize) -> isize`
-   = help: if the expected type is due to type inference, cast the expected `fn` to a function pointer: `foo::<u8> as fn(isize) -> isize`


When we expect a FnDef, and find a FnPtr, we should mention that the user can use an as to turn the def into a pointer. Since the span will be pointing at the ptr, not the def, this can't be a suggestion, but it's still worth noting.

compiler-errors · 2023-01-24T22:38:03Z

I'm inclined to merge this as-is since it's been quite a lot of back-and-forth and I don't want to keep you waiting. If you're free, then please work on the follow-ups in an additional PR, and if you're not, let me know so I can make the changes myself 😃

@bors r+ rollup

bors · 2023-01-24T22:38:05Z

📌 Commit 1e22280 has been approved by compiler-errors

It is now in the queue for this repository.

mattjperez · 2023-01-24T22:41:23Z

Not a problem! I appreciate all the help and patience you've given with this one. I can make the changes in a follow-up PR.

Rollup of 9 pull requests Successful merges: - rust-lang#105552 (Add help message about function pointers) - rust-lang#106583 (Suggest coercion of `Result` using `?`) - rust-lang#106767 (Allow setting CFG_DISABLE_UNSTABLE_FEATURES to 0) - rust-lang#106823 (Allow fmt::Arguments::as_str() to return more Some(_).) - rust-lang#107166 (rustc_metadata: Support non-`Option` nullable values in metadata tables) - rust-lang#107213 (Add suggestion to remove if in let..else block) - rust-lang#107223 (`sub_ptr()` is equivalent to `usize::try_from().unwrap_unchecked()`, not `usize::from().unwrap_unchecked()`) - rust-lang#107227 (`new_outside_solver` -> `evaluate_root_goal`) - rust-lang#107232 (rustdoc: simplify settings popover DOM, CSS, JS) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

…s, r=compiler-errors Improve fn pointer notes continuation of rust-lang#105552 r? `@compiler-errors`

rustbot assigned cjgillot Dec 11, 2022

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 Dec 11, 2022

rustbot assigned TaKO8Ki and unassigned cjgillot Dec 11, 2022

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 11, 2022

compiler-errors reviewed Dec 11, 2022

View reviewed changes

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

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

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

This comment has been minimized.

Sign in to view

compiler-errors reviewed Dec 12, 2022

View reviewed changes

mattjperez force-pushed the add-incompatible-types-note branch 3 times, most recently from 476adb8 to 59ebb65 Compare December 19, 2022 21:59

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 (such as code changes or more information) from the author. labels Dec 19, 2022

compiler-errors reviewed Dec 19, 2022

View reviewed changes

mattjperez force-pushed the add-incompatible-types-note branch 2 times, most recently from 305abb8 to bfdfb85 Compare December 20, 2022 04:14

mattjperez requested a review from compiler-errors December 30, 2022 17:02

rustbot assigned compiler-errors and unassigned TaKO8Ki Jan 9, 2023

compiler-errors mentioned this pull request Jan 9, 2023

Unsufficient report when encountering a mismatch between fn pointer and fn item #106652

Closed

compiler-errors reviewed Jan 11, 2023

View reviewed changes

mattjperez force-pushed the add-incompatible-types-note branch from 5588693 to 7ae4704 Compare January 21, 2023 20:52

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 (such as code changes or more information) from the author. labels Jan 21, 2023

compiler-errors reviewed Jan 22, 2023

View reviewed changes

compiler/rustc_infer/src/infer/error_reporting/suggest.rs Outdated Show resolved Hide resolved

mattjperez force-pushed the add-incompatible-types-note branch 2 times, most recently from 29caebf to a67f653 Compare January 24, 2023 15:52

compiler-errors reviewed Jan 24, 2023

View reviewed changes

tests/ui/issues/issue-59488.stderr Outdated Show resolved Hide resolved

mattjperez force-pushed the add-incompatible-types-note branch 3 times, most recently from 9d3785b to 0e6e5d6 Compare January 24, 2023 17:37

compiler-errors reviewed Jan 24, 2023

View reviewed changes

tests/ui/reify-intrinsic.stderr Outdated Show resolved Hide resolved

mattjperez force-pushed the add-incompatible-types-note branch from 0e6e5d6 to 1e22280 Compare January 24, 2023 19:03

compiler-errors approved these changes Jan 24, 2023

View reviewed changes

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 Jan 24, 2023

mattjperez mentioned this pull request Jan 25, 2023

Improve fn pointer notes #107287

Merged

Dylan-DPC mentioned this pull request Jan 25, 2023

Rollup of 9 pull requests #107290

Merged

bors merged commit c0930c4 into rust-lang:master Jan 25, 2023

rustbot added this to the 1.69.0 milestone Jan 25, 2023

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 26, 2023

Rollup merge of rust-lang#107287 - mattjperez:improve-fn-pointer-note…

f2f1234

…s, r=compiler-errors Improve fn pointer notes continuation of rust-lang#105552 r? `@compiler-errors`

mattjperez deleted the add-incompatible-types-note branch January 26, 2023 13:03

Add help message about function pointers #105552

Add help message about function pointers #105552

Uh oh!

Conversation

mattjperez commented Dec 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 11, 2022

Uh oh!

mattjperez commented Dec 11, 2022

Uh oh!

mattjperez commented Dec 11, 2022

Uh oh!

TaKO8Ki commented Dec 11, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattjperez commented Dec 19, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented Jan 9, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattjperez Jan 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattjperez commented Jan 21, 2023

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattjperez commented Jan 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Jan 24, 2023

Uh oh!

bors commented Jan 24, 2023

Uh oh!

mattjperez commented Jan 24, 2023

Uh oh!

Reviewers

Assignees

Labels

mattjperez commented Dec 11, 2022 •

edited

Loading

mattjperez Jan 18, 2023 •

edited

Loading

mattjperez commented Jan 24, 2023 •

edited

Loading