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 more information to impl Trait error #92806

Merged
merged 2 commits into from Feb 19, 2022

Conversation

compiler-errors
Copy link
Member

Fixes #92458

Let me know if I went overboard here, or if the suggestions could use some refinement.

r? @estebank
Feel free to reassign to someone else

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 12, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 12, 2022
@compiler-errors
Copy link
Member Author

Alternatively we can disambiguate #92458 by just adding that word "inherent", without adding these ImplTraitPositions everywhere..

@bors
Copy link
Contributor

bors commented Feb 8, 2022

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

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

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

Couple of nitpicks, r=me after rebasing

Comment on lines 1403 to 1406
"`impl Trait` not allowed outside of \
function and inherent method return types",
);
err.note(&format!("found `impl Trait` in {}", position));
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm concerned about "inherent" in this wording not being too clear. "associated functions" doesn't communicate the same meaning, sadly. Either way, I would likely change this to be

Suggested change
"`impl Trait` not allowed outside of \
function and inherent method return types",
);
err.note(&format!("found `impl Trait` in {}", position));
"`impl Trait` only allowed in function and inherent method return types, not in {}", position
);

fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let name = match self {
ImplTraitPosition::Path => "path",
ImplTraitPosition::Variable => "variable",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ImplTraitPosition::Variable => "variable",
ImplTraitPosition::Variable => "variable binding",

@estebank estebank 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 Feb 17, 2022
@compiler-errors
Copy link
Member Author

Rebased and addressed comments, ready for r+

@compiler-errors
Copy link
Member Author

@rustbot ready

@rustbot 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 Feb 18, 2022
@estebank
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Feb 18, 2022

📌 Commit 207fb5f has been approved by estebank

@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 Feb 18, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 19, 2022
…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
@bors bors merged commit 5c08c39 into rust-lang:master Feb 19, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 19, 2022
@compiler-errors compiler-errors deleted the better-impl-trait-deny branch April 7, 2022 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

using impl Trait on a return type as part of a trait declaration results in a misleading error
5 participants