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

Nest the impl Trait existential item inside the return type #54741

Merged
merged 2 commits into from
Oct 5, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Oct 2, 2018

fixes #54045

r? @cramertj

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 2, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@cramertj
Copy link
Member

cramertj commented Oct 2, 2018

Nice!

@bors r+

@bors
Copy link
Contributor

bors commented Oct 2, 2018

📌 Commit c3151c6 has been approved by cramertj

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

bors commented Oct 5, 2018

⌛ Testing commit c3151c6 with merge 2155f27...

bors added a commit that referenced this pull request Oct 5, 2018
Nest the `impl Trait` existential item inside the return type

fixes #54045

r? @cramertj
@bors
Copy link
Contributor

bors commented Oct 5, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: cramertj
Pushing 2155f27 to master...

@bors bors merged commit c3151c6 into rust-lang:master Oct 5, 2018
@bors bors mentioned this pull request Oct 5, 2018
Manishearth added a commit to rust-lang/rust-clippy that referenced this pull request Oct 5, 2018
Manishearth added a commit to rust-lang/rust-clippy that referenced this pull request Oct 6, 2018
///
/// The generic arg list are the lifetimes (and in the future possibly parameters) that are
/// actually bound on the `impl Trait`.
Def(ItemId, HirVec<GenericArg>),
Copy link
Member

Choose a reason for hiding this comment

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

I want something like this in general, maybe {...} block syntax for types?
Then -> impl Trait would be -> { existential type X: Trait; X }.
cc @Centril @nikomatsakis

Copy link
Contributor

Choose a reason for hiding this comment

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

In other words this would be legal?

type Foo = { u8 };

Not sure why it is useful tho...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isn't that essentially generic modules? (but unnamed)

Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk I guess only if you can depend on parameters in scope.

@Centril for the same reason we have it for expressions :P.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess only if you can depend on parameters in scope.

which impl trait always does

Copy link
Contributor

Choose a reason for hiding this comment

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

@eddyb in exprs it exists for some sort of encapsulation purposes... I suppose you could do some sort of encapsulation of type aliases... but we should tread carefully here, there might be ambiguity in store.

PS: we should move this conversation elsewhere into a fresh issue / internals instead of comments on a merged PR (totally not discoverable...).

Copy link
Member

Choose a reason for hiding this comment

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

@Centril

I suppose you could do some sort of encapsulation of type aliases

Module-based encapsulation is how existential type X: Trait; works today.

Copy link
Contributor

Choose a reason for hiding this comment

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

@cramertj I meant something like this:

type Foo = {
    type ComplexType = Bar<Baz<u8>>;
    Vec<ComplexType>
};

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in #[allow(deprecated)] for impl Trait return type
6 participants