Skip to content

Error on nested impl Trait and path projections from impl Trait #48084

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 5 commits into from
Feb 24, 2018

Conversation

cramertj
Copy link
Member

@cramertj cramertj commented Feb 9, 2018

@kennytm kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 9, 2018
@bors
Copy link
Collaborator

bors commented Feb 12, 2018

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

@bors bors 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 12, 2018
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

New test, new error message, but seems good

for type_binding in &params.bindings {
// Type bindings such as `Item=impl Debug` in `Iterator<Item=Debug>`
// are allowed to contain nested `impl Trait`.
self.with_impl_trait(None, |this| visit::walk_ty(this, &type_binding.ty));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this code would permit this

<dyn Iterator<Item = impl Debug> as Iterator>::Item

which ought not to be allowed

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, probably not, because that is handled by a totally distinct visitor, I see

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test for it, anyway?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

struct_span_err!(self.session, t.span, E0666,
"nested `impl Trait` is not allowed")
.span_label(outer_impl_trait, "outer `impl Trait`")
.span_label(t.span, "devilishly nested `impl Trait` here")
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't "devilishly" should be in our error message

Copy link
Member Author

Choose a reason for hiding this comment

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

No love for the E0666 easter egg 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@nikomatsakis
Copy link
Contributor

Travis is unhappy:

https://travis-ci.org/rust-lang/rust/builds/341238329#L1665-L1686

The ui/error-codes/E0657.rs test is failing.

@nikomatsakis
Copy link
Contributor

r=me once travis is happy

@cramertj
Copy link
Member Author

@bors r=nikomatsakis

@bors
Copy link
Collaborator

bors commented Feb 15, 2018

📌 Commit 9e9c55f has been approved by nikomatsakis

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 15, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 19, 2018
…atsakis

Error on nested impl Trait and path projections from impl Trait

cc rust-lang#34511

r? @nikomatsakis
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 20, 2018
bors added a commit that referenced this pull request Feb 20, 2018
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
…atsakis

Error on nested impl Trait and path projections from impl Trait

cc rust-lang#34511

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 24, 2018
Rollup of 15 pull requests

- Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402
- Failed merges:
Manishearth added a commit to Manishearth/rust that referenced this pull request Feb 24, 2018
…atsakis

Error on nested impl Trait and path projections from impl Trait

cc rust-lang#34511

r? @nikomatsakis
bors added a commit that referenced this pull request Feb 24, 2018
Rollup of 15 pull requests

- Successful merges: #47987, #48056, #48061, #48084, #48143, #48185, #48206, #48208, #48232, #48246, #48258, #48317, #48353, #48356, #48402
- Failed merges:
@bors bors merged commit 9e9c55f into rust-lang:master Feb 24, 2018
@cramertj cramertj deleted the impl-trait-errors branch February 24, 2018 23:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants