Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering #66197

Centril · 2019-11-07T19:22:02Z

We currently have a hack in the form of ast::{ItemKind, ImplItemKind}::OpaqueTy which is constructed literally when you write type Alias = impl Trait; but not e.g. type Alias = Vec<impl Trait>;. Per rust-lang/rfcs#2515, this needs to change to allow impl Trait in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, TyKind::opaque_top_hack is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported.

r? @varkor

petrochenkov · 2019-11-09T17:17:59Z

src/librustc_resolve/build_reduced_graph.rs

So, I noticed today that DefKind::OpaqueTy and DefKind::AssocOpaqueTy exist despite the existential type syntax being removed.

Is it possible to remove them after this PR or it will require reworking lowering as well?

I'm not sure. We might need something like:

type Foo = (impl Bar, impl Baz); // In AST

=> (HIR)

opaque type _0: Bar; // DefKind::OpaqueTy opaque type _1: Baz; // DefKind::OpaqueTy type Foo = ( // DefKind::TyAlias _0, // TyKind::Def(..) _1 // TyKind::Def(..) );

From my point of view: as long we still have something like opaque type in HIR still, that's good enough for my upcoming PR. I'd rather not remove that for a while yet.

We won't get rid of opaque types in HIR (they will always just arise from type aliases).

Yeah, that's what I thought. :-)

varkor · 2019-11-11T15:53:52Z

Looks good to me, thanks!

@bors r+

bors · 2019-11-11T15:53:54Z

📌 Commit c43e4d1 has been approved by varkor

bors · 2019-11-13T18:32:59Z

⌛ Testing commit c43e4d1 with merge a2ae41c...

@varkor

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering We currently have a hack in the form of `ast::{ItemKind, ImplItemKind}::OpaqueTy` which is constructed literally when you write `type Alias = impl Trait;` but not e.g. `type Alias = Vec<impl Trait>;`. Per rust-lang/rfcs#2515, this needs to change to allow `impl Trait` in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, `TyKind::opaque_top_hack` is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported. r? @varkor

Centril · 2019-11-14T13:01:26Z

Ran ./x.py -i test src/test/ui --stage 1 --bless --pass check --compare-mode=nll

@bors r=varkor

bors · 2019-11-14T13:01:27Z

📌 Commit 03cf0d7 has been approved by varkor

@varkor

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering We currently have a hack in the form of `ast::{ItemKind, ImplItemKind}::OpaqueTy` which is constructed literally when you write `type Alias = impl Trait;` but not e.g. `type Alias = Vec<impl Trait>;`. Per rust-lang/rfcs#2515, this needs to change to allow `impl Trait` in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, `TyKind::opaque_top_hack` is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported. r? @varkor

@varkor

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering We currently have a hack in the form of `ast::{ItemKind, ImplItemKind}::OpaqueTy` which is constructed literally when you write `type Alias = impl Trait;` but not e.g. `type Alias = Vec<impl Trait>;`. Per rust-lang/rfcs#2515, this needs to change to allow `impl Trait` in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, `TyKind::opaque_top_hack` is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported. r? @varkor

bors · 2019-11-15T12:58:45Z

⌛ Testing commit 03cf0d7 with merge 29095e1395f6f446330750531b65536d9066d6f7...

@varkor

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering We currently have a hack in the form of `ast::{ItemKind, ImplItemKind}::OpaqueTy` which is constructed literally when you write `type Alias = impl Trait;` but not e.g. `type Alias = Vec<impl Trait>;`. Per rust-lang/rfcs#2515, this needs to change to allow `impl Trait` in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, `TyKind::opaque_top_hack` is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported. r? @varkor

Centril · 2019-11-15T13:13:11Z

@bors retry rolled up.

@varkor

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering We currently have a hack in the form of `ast::{ItemKind, ImplItemKind}::OpaqueTy` which is constructed literally when you write `type Alias = impl Trait;` but not e.g. `type Alias = Vec<impl Trait>;`. Per rust-lang/rfcs#2515, this needs to change to allow `impl Trait` in nested positions. This PR achieves this change for the syntactic aspect but not the semantic one, which will require changes in lowering and def collection. In the interim, `TyKind::opaque_top_hack` is introduced to avoid knock-on changes in lowering, collection, and resolve. These hacks can then be removed and fixed one by one until the desired semantics are supported. r? @varkor

@ghost

Rollup of 4 pull requests Successful merges: - #66197 (Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering) - #66429 (Add a regression test for #62524) - #66435 (Correct `const_in_array_repeat_expressions` feature name) - #66443 (Port erased cleanup) Failed merges: r? @ghost

bors · 2019-11-15T16:57:31Z

⌛ Testing commit 03cf0d7 with merge 1bd30ce...

Centril added the F-type_alias_impl_trait `#[feature(type_alias_impl_trait)]` label Nov 7, 2019

rust-highfive assigned varkor Nov 7, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 7, 2019

petrochenkov assigned petrochenkov and unassigned petrochenkov Nov 7, 2019

Centril force-pushed the transparent-ast branch from f1f191b to 9ce6457 Compare November 8, 2019 08:28

This comment has been minimized.

Sign in to view

Centril force-pushed the transparent-ast branch from 9ce6457 to 4ee926d Compare November 9, 2019 01:16

petrochenkov reviewed Nov 9, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

Centril force-pushed the transparent-ast branch from 4ee926d to c43e4d1 Compare November 11, 2019 05:20

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 Nov 11, 2019

bors mentioned this pull request Nov 13, 2019

Add a HIR pass to check consts for if, loop, etc. #66170

Merged

This comment has been minimized.

Sign in to view

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 13, 2019

Centril added 8 commits November 14, 2019 13:40

TAIT: remove OpaqueTy in AST.

6a49b52

TAIT: feature gate recursive locations

6d8e300

TAIT: parse recursively instead of hack.

0e8e176

TAIT: use hack in ->HIR to avoid more changes

aa6a72f

TAIT: remove redundant check from ast_validation

e31d75c

TAIT: adjust save-analysis

89b5907

TAIT: adjust resolve

75aaa85

TAIT: --bless some span changes for the better

8b663ec

TAIT: adjust tests

03cf0d7

Centril force-pushed the transparent-ast branch from c43e4d1 to 03cf0d7 Compare November 14, 2019 13:00

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 Nov 14, 2019

tmandry mentioned this pull request Nov 14, 2019

Rollup of 8 pull requests #66417

Closed

tmandry mentioned this pull request Nov 14, 2019

Rollup of 9 pull requests #66421

Closed

Centril mentioned this pull request Nov 15, 2019

Rollup of 5 pull requests #66446

Closed

tmandry mentioned this pull request Nov 15, 2019

Rollup of 4 pull requests #66449

Merged

bors merged commit 03cf0d7 into rust-lang:master Nov 15, 2019

Centril deleted the transparent-ast branch November 15, 2019 17:14

Push ast::{ItemKind, ImplItemKind}::OpaqueTy hack down into lowering #66197

Push ast::{ItemKind, ImplItemKind}::OpaqueTy hack down into lowering #66197

Uh oh!

Conversation

Centril commented Nov 7, 2019

Uh oh!

This comment has been minimized.

petrochenkov Nov 9, 2019

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexreg Nov 10, 2019

Choose a reason for hiding this comment

Uh oh!

varkor Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

alexreg Nov 11, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

varkor commented Nov 11, 2019

Uh oh!

bors commented Nov 11, 2019

Uh oh!

bors commented Nov 13, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

Centril commented Nov 14, 2019

Uh oh!

bors commented Nov 14, 2019

Uh oh!

bors commented Nov 15, 2019

Uh oh!

Centril commented Nov 15, 2019

Uh oh!

bors commented Nov 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering #66197

Push `ast::{ItemKind, ImplItemKind}::OpaqueTy` hack down into lowering #66197

Centril Nov 10, 2019 •

edited

Loading