Skip to content

Further improve impl Trait/dyn Trait suggestions - #68522

Merged
bors merged 6 commits into
rust-lang:masterfrom
estebank:impl-trait-sugg-2
Jan 26, 2020
Merged

Further improve impl Trait/dyn Trait suggestions#68522
bors merged 6 commits into
rust-lang:masterfrom
estebank:impl-trait-sugg-2

Conversation

@estebank

@estebankestebank commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

After reading Returning Trait Objects by Bryce Fisher-Fleig, I noticed that#68195 had a few bugs due to not ignoring ty::Error.

  • Account for ty::Error.
  • Account for if/else and match blocks when pointing at return types and referencing their types.
  • Increase the multiline suggestion output from 6 lines to 20.

@estebank
Account for ty::Error when suggesting impl Trait or Box<dyn Trait>
55dce72
@estebank
Use more accurate return path spans
d14a323
No longer suggest `Box::new(if foo { Type1 } else { Type2 })`, instead
suggesting `if foo { Box::new(Type1) } else { Box::new(Type2) }`.
@estebank
Increase suggestion code window from 6 lines to 20
34d51b3
@rust-highfive

rust-highfive commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

r? @eddyb

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

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2020
@estebank

estebank commented Jan 24, 2020

Copy link
Copy Markdown
ContributorAuthor

CC @oli-obk

@oli-obk

oli-obk commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

r? @oli-obk

@rust-highfiverust-highfive assigned oli-obk and unassigned eddybJan 24, 2020
Centril
Centril reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/librustc/traits/error_reporting/suggestions.rs
Centril
Centril reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/librustc/traits/error_reporting/suggestions.rs
oli-obk
oli-obk reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/librustc/traits/error_reporting/suggestions.rs Outdated
oli-obk
oli-obk reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/librustc/traits/error_reporting/suggestions.rs Outdated
oli-obk
oli-obk reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/test/ui/error-codes/E0746.rs Outdated
oli-obk
oli-obk reviewed Jan 24, 2020
View reviewed changes
Comment threadsrc/librustc_errors/emitter.rs Outdated
@oli-obk

oli-obk commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jan 24, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 600e385 has been approved by oli-obk

@borsbors 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, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jan 25, 2020
@Dylan-DPC
Rollup merge ofrust-lang#68522- estebank:impl-trait-sugg-2, r=oli-obk
a24c77d
Further improve `impl Trait`/`dyn Trait` suggestions
After reading [_Returning Trait Objects_ by Bryce Fisher-Fleig](https://bryce.fisher-fleig.org/blog/returning-trait-objects/), [I noticed that](https://www.reddit.com/r/rust/comments/esueur/returning_trait_objects/ffczl4k/) rust-lang#68195 had a few bugs due to not ignoring `ty::Error`.
- Account for `ty::Error`.
- Account for `if`/`else` and `match` blocks when pointing at return types and referencing their types.
- Increase the multiline suggestion output from 6 lines to 20.
@Dylan-DPC-zzDylan-DPC-zz mentioned this pull request Jan 25, 2020
Closed
oli-obk
oli-obk reviewed Jan 25, 2020
View reviewed changes
Comment threadsrc/librustc_errors/emitter.rs
@oli-obk

oli-obk commented Jan 25, 2020

Copy link
Copy Markdown
Contributor

@bors r-

@borsbors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 25, 2020
@estebank

estebank commented Jan 25, 2020

Copy link
Copy Markdown
ContributorAuthor

@bors r=oli-obk
reverted the suggestion output change

@bors

bors commented Jan 25, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 16709f0 has been approved by oli-obk

@borsbors 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 Jan 25, 2020
@bors

bors commented Jan 26, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 16709f0 with merge 3d8778d...

bors added a commit that referenced this pull request Jan 26, 2020
@bors
Auto merge of#68522- estebank:impl-trait-sugg-2, r=oli-obk
3d8778d
Further improve `impl Trait`/`dyn Trait` suggestions
After reading [_Returning Trait Objects_ by Bryce Fisher-Fleig](https://bryce.fisher-fleig.org/blog/returning-trait-objects/), [I noticed that](https://www.reddit.com/r/rust/comments/esueur/returning_trait_objects/ffczl4k/) #68195 had a few bugs due to not ignoring `ty::Error`.
- Account for `ty::Error`.
- Account for `if`/`else` and `match` blocks when pointing at return types and referencing their types.
- Increase the multiline suggestion output from 6 lines to 20.
@bors

bors commented Jan 26, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 3d8778d to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jan 26, 2020
@bors
bors merged commit 16709f0 into rust-lang:masterJan 26, 2020
@borsbors mentioned this pull request Jan 26, 2020
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obkoli-obkoli-obk left review comments
+1 more reviewer
@CentrilCentrilCentril left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@oli-obkoli-obk

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@estebank@rust-highfive@oli-obk@bors@Centril@eddyb