Suggest `impl Trait` return type when incorrectly using a generic return type by Noratrieb · Pull Request #89892 · rust-lang/rust

Noratrieb · 2021-10-14T20:02:34Z

When there is a type mismatch error and the return type is generic, and that generic parameter is not used in the function parameters, suggest replacing that generic with the impl Trait syntax.

r? @estebank

rust-highfive · 2021-10-14T20:02:37Z

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

Please see the contribution instructions for more information.

Noratrieb · 2021-10-14T20:04:54Z

This is my first PR, so I hope I did everything correctly!

src/test/ui/return/return-impl-trait.stderr

C0RR1T · 2021-10-15T06:03:04Z

As a new Rust programmer, I really want this feature

Noratrieb · 2021-10-19T06:46:44Z

I'm done with the changes, you can review it again if you have time

Noratrieb · 2021-10-26T20:24:15Z

What's the status on this PR? @estebank

jyn514

This is a neat idea :) it looks in pretty good shape, just a few suggestions.

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

src/test/ui/return/return-impl-trait.rs

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

Noratrieb · 2021-11-10T18:57:45Z

I added the notes, also added // run-rustfix to test the suggestions, and squashed all commits into one, since the change is pretty small.

jyn514 · 2021-11-10T21:20:30Z

@Nilstrieb you need to add the .fixed file I think - you might be able to generate it with --bless --run-rustfix.

Noratrieb · 2021-11-10T21:33:53Z

It does have the fixed code, but the problem is that this fixed code is not compiling successfully because of bad_echo. How can I allow this?

jyn514 · 2021-11-10T21:34:40Z

@Nilstrieb I'd suggest making the rustfix test separate from the rest of the tests.

Noratrieb · 2021-11-10T21:41:39Z

I've squashed it again since this is hopefully the last change

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

src/test/ui/return/return-impl-trait.rs

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

Noratrieb · 2021-12-15T09:14:47Z

Thank you for your review, I'll implement this next week!

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

jackh726 · 2022-01-09T22:05:58Z

r? @jackh726

jackh726

Sorry, more reviews.

Also, can you squash the commits?

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

jackh726 · 2022-01-14T23:14:39Z

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

This doesn't necessarily cover cases where T appears in the bounds, right? So like Option<()>: Trait<T>

Hmm, I wasn't able to figure out how I'd do that. Do you have an idea?

Maybe instead of making contains be on Ty, you can make it a visitor. Then just visit both the ty and bounds.

For the contains I was able to use the visitor on Ty. How should I visit the PolyTraitRef in the bounds? I found hir::intravisit but wasn't able to find a good way to use it.

Sorry, thought I replied to this before.

There is a hir visitor: https://doc.rust-lang.org/stable/nightly-rustc/rustc_hir/intravisit/trait.Visitor.html

Alternatively, you could lower the trait ref.

I'm also fine it you just want to add this as a test case with a FIXME. This is just suggestion after all and doesn't have to he "perfect"

I'll just leave a fixme in a test 👍

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs

Noratrieb · 2022-01-15T15:31:03Z

I'll squash it once it's done.

bors · 2022-01-18T14:12:50Z

☔ The latest upstream changes (presumably #87648) made this pull request unmergeable. Please resolve the merge conflicts.

Noratrieb · 2022-01-31T19:47:27Z

Finally did it, I wanted to have TyS::contains as a separate commit at first, but after a little git accident I decided to just squash them anyways

bors · 2022-02-15T14:43:29Z

☔ The latest upstream changes (presumably #93148) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726 · 2022-02-15T16:51:04Z

Sorry I didn't get to this before; r=me here after rebase

jackh726 · 2022-02-15T16:51:27Z

@bors delegate+

(just do r=jackh726)

bors · 2022-02-15T16:51:29Z

✌️ @Nilstrieb can now approve this pull request

Address rust-lang#85991 Suggest the `impl Trait` return type syntax if the user tried to return a generic parameter and we get a type mismatch The suggestion is not emitted if the param appears in the function parameters, and only get the bounds that actually involve `T: ` directly It also checks whether the generic param is contained in any where bound (where it isn't the self type), and if one is found (like `Option<T>: Send`), it is not suggested. This also adds `TyS::contains`, which recursively vistits the type and looks if the other type is contained anywhere

Noratrieb · 2022-02-18T20:33:35Z

@bors r=jackh726

bors · 2022-02-18T20:33:36Z

📌 Commit 4bed748 has been approved by jackh726

…askrgr Rollup of 10 pull requests Successful merges: - rust-lang#89892 (Suggest `impl Trait` return type when incorrectly using a generic return type) - rust-lang#91675 (Add MemTagSanitizer Support) - rust-lang#92806 (Add more information to `impl Trait` error) - rust-lang#93497 (Pass `--test` flag through rustdoc to rustc so `#[test]` functions can be scraped) - rust-lang#93814 (mips64-openwrt-linux-musl: correct soft-foat) - rust-lang#93847 (kmc-solid: Use the filesystem thread-safety wrapper) - rust-lang#93877 (asm: Allow the use of r8-r14 as clobbers on Thumb1) - rust-lang#93892 (Only mark projection as ambiguous if GAT substs are constrained) - rust-lang#93915 (Implement --check-cfg option (RFC 3013), take 2) - rust-lang#93953 (Add the `known-bug` test directive, use it, and do some cleanup) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

rust-highfive assigned estebank Oct 14, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 14, 2021

This comment has been minimized.

Sign in to view

estebank reviewed Oct 14, 2021

View reviewed changes

src/test/ui/return/return-impl-trait.stderr Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 21, 2021

jyn514 reviewed Nov 4, 2021

View reviewed changes

Noratrieb force-pushed the suggest-return-impl-trait branch 2 times, most recently from 96573f1 to 4572455 Compare November 10, 2021 18:56

This comment has been minimized.

Sign in to view

Noratrieb force-pushed the suggest-return-impl-trait branch from 4572455 to eafa4bf Compare November 10, 2021 21:41

jackh726 requested changes Dec 4, 2021

View reviewed changes

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs Outdated Show resolved Hide resolved

src/test/ui/return/return-impl-trait.rs Outdated Show resolved Hide resolved

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs Outdated Show resolved Hide resolved

Noratrieb force-pushed the suggest-return-impl-trait branch from eafa4bf to 606f6e2 Compare December 29, 2021 09:43

This comment has been minimized.

Sign in to view

jackh726 reviewed Jan 9, 2022

View reviewed changes

compiler/rustc_typeck/src/check/fn_ctxt/suggestions.rs Outdated Show resolved Hide resolved

rust-highfive assigned jackh726 and unassigned estebank Jan 9, 2022

jackh726 requested changes Jan 14, 2022

View reviewed changes

jackh726 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 Jan 30, 2022

This comment has been minimized.

Sign in to view

Noratrieb force-pushed the suggest-return-impl-trait branch 2 times, most recently from 5d2ddbd to 3005d4d Compare January 31, 2022 07:29

This comment has been minimized.

Sign in to view

Noratrieb force-pushed the suggest-return-impl-trait branch from 3005d4d to 2400a75 Compare January 31, 2022 19:46

Noratrieb requested a review from jackh726 February 4, 2022 08:54

Noratrieb force-pushed the suggest-return-impl-trait branch from 2400a75 to 3e6e9df Compare February 18, 2022 19:17

This comment has been minimized.

Sign in to view

Noratrieb force-pushed the suggest-return-impl-trait branch from 3e6e9df to 4bed748 Compare February 18, 2022 19:40

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 18, 2022

matthiaskrgr mentioned this pull request Feb 18, 2022

Rollup of 10 pull requests #94134

Merged

bors merged commit f8b83a2 into rust-lang:master Feb 19, 2022

rustbot added this to the 1.60.0 milestone Feb 19, 2022

jackh726 mentioned this pull request Feb 19, 2022

Suggest using -> impl Trait when returning type parameter -> T where T: Trait #85991

Closed

Noratrieb deleted the suggest-return-impl-trait branch February 19, 2022 09:50

Uh oh!

Conversation

Noratrieb commented Oct 14, 2021

Uh oh!

rust-highfive commented Oct 14, 2021

Uh oh!

Noratrieb commented Oct 14, 2021

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

C0RR1T commented Oct 15, 2021

Uh oh!

This comment has been minimized.

Noratrieb commented Oct 19, 2021

Uh oh!

Noratrieb commented Oct 26, 2021

Uh oh!

jyn514 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Noratrieb commented Nov 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

jyn514 commented Nov 10, 2021

Uh oh!

Noratrieb commented Nov 10, 2021

Uh oh!

jyn514 commented Nov 10, 2021

Uh oh!

Noratrieb commented Nov 10, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Noratrieb commented Dec 15, 2021

Uh oh!

This comment has been minimized.

Uh oh!

jackh726 commented Jan 9, 2022

Uh oh!

jackh726 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jackh726 Jan 14, 2022

Choose a reason for hiding this comment

Uh oh!

Noratrieb Jan 15, 2022

Choose a reason for hiding this comment

Uh oh!

jackh726 Jan 15, 2022

Choose a reason for hiding this comment

Uh oh!

Noratrieb Jan 16, 2022

Choose a reason for hiding this comment

Uh oh!

jackh726 Jan 30, 2022

Choose a reason for hiding this comment

Uh oh!

Noratrieb Jan 31, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Noratrieb commented Jan 15, 2022

Uh oh!

bors commented Jan 18, 2022

Uh oh!

This comment has been minimized.

This comment has been minimized.

Noratrieb commented Jan 31, 2022

Noratrieb commented Nov 10, 2021 •

edited

Loading