Skip to content
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

Jump to bottom

ty/walk: iterate GenericArgs instead of Tys. #70164

Merged
merged 6 commits into from
Apr 7, 2020
Merged

ty/walk: iterate GenericArgs instead of Tys. #70164

merged 6 commits into from
Apr 7, 2020

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 19, 2020 •
edited
Loading

Before this PR, Ty::walk only iterated over Tys, but that's becoming an increasing problem with const generics, as ty::Consts in Substs are missed by it.

By working with GenericArg instead, we can handle both Tys and ty::Consts, but also ty::Regions, which used to require ad-hoc mechanisms such as push_regions.

I've also removed TraitRef::input_types, as it's both long obsolete, and easy to misuse.

@rust-highfive

This comment has been minimized.

# to view
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 19, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 19, 2020

r? @nikomatsakis cc @matthewjasper @varkor @yodaldevoid

@eddyb
Copy link
Member Author

eddyb commented Mar 19, 2020

I'm doing a perf run with just commit, to assess the impact of collecting ty::Regions and ty::Consts besides just Tys, without exposing non-Tys in the Ty::walk API:

ty/walk: keep track of GenericArgs on the stack, instead of Tys.

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 19, 2020

⌛ Trying commit 27d6047f0fc787370b110688389204e2c4c57bc6 with merge e8f55d1b486e3355f5cb9b8252d069943e421ac0...

@eddyb eddyb mentioned this pull request Mar 19, 2020
Merged
@varkor varkor added the F-const_generics `#![feature(const_generics)]` label Mar 19, 2020
@bors
Copy link
Contributor

bors commented Mar 19, 2020

☀️ Try build successful - checks-azure
Build commit: e8f55d1b486e3355f5cb9b8252d069943e421ac0 (e8f55d1b486e3355f5cb9b8252d069943e421ac0)

@rust-timer
Copy link
Collaborator

Queued e8f55d1b486e3355f5cb9b8252d069943e421ac0 with parent f4c675c, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit e8f55d1b486e3355f5cb9b8252d069943e421ac0, comparison URL.

@eddyb
Copy link
Member Author

eddyb commented Mar 20, 2020

Looks harmless, I guess? I'll finish the PR assuming the first commit is acceptable.

@eddyb eddyb mentioned this pull request Mar 20, 2020
Closed
@eddyb eddyb mentioned this pull request Mar 21, 2020
Merged
@eddyb eddyb force-pushed the walk-generic-arg branch from 27d6047 to 135ee05 Compare March 22, 2020 02:08
@eddyb

This comment has been minimized.

# to view
@eddyb eddyb added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Mar 22, 2020
@eddyb eddyb force-pushed the walk-generic-arg branch from 135ee05 to 7be2176 Compare March 22, 2020 05:35
@eddyb eddyb mentioned this pull request Mar 22, 2020
Merged
@eddyb

This comment has been minimized.

# to view
@rust-highfive

This comment has been minimized.

# to view
@eddyb eddyb force-pushed the walk-generic-arg branch from f76e2f1 to 10d72f0 Compare March 23, 2020 03:45
@eddyb eddyb mentioned this pull request Mar 23, 2020
Closed
@eddyb eddyb changed the title [WIP] ty/walk: iterate GenericArgs instead of Tys. ty/walk: iterate GenericArgs instead of Tys. Mar 23, 2020
@eddyb eddyb force-pushed the walk-generic-arg branch from 0ca1e7a to 626abc7 Compare April 6, 2020 19:59
@eddyb
Copy link
Member Author

eddyb commented Apr 6, 2020

@bors r=nikomatsakis rollup=never (just in case we get another surprise)

@bors
Copy link
Contributor

bors commented Apr 6, 2020

📌 Commit 626abc7 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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 6, 2020
@mark-i-m mark-i-m mentioned this pull request Apr 6, 2020
Closed
8 tasks
@Dylan-DPC-zz
Copy link

@bors p=1

@mark-i-m mark-i-m mentioned this pull request Apr 7, 2020
Merged
@bors
Copy link
Contributor

bors commented Apr 7, 2020

⌛ Testing commit 626abc7 with merge 209b2be...

@bors
Copy link
Contributor

bors commented Apr 7, 2020

☀️ Test successful - checks-azure
Approved by: nikomatsakis
Pushing 209b2be to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 7, 2020
@bors bors merged commit 209b2be into rust-lang:master Apr 7, 2020
@rust-highfive rust-highfive mentioned this pull request Apr 7, 2020
Closed
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #70164!

Tested on commit 209b2be.
Direct link to PR: #70164

💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Apr 7, 2020
@rust-highfive
📣 Toolstate changed by rust-lang/rust#70164!
11c286c 
Tested on commit rust-lang/rust@209b2be.
Direct link to PR: <rust-lang/rust#70164>

💔 clippy-driver on windows: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
💔 clippy-driver on linux: test-pass → build-fail (cc @mcarton @oli-obk @Manishearth @flip1995 @yaahc @phansch @llogiq).
This was referenced Apr 7, 2020
Merged
Merged
@eddyb eddyb deleted the walk-generic-arg branch April 7, 2020 09:45
@JohnTitor JohnTitor mentioned this pull request Apr 8, 2020
Merged
@Centril Centril mentioned this pull request Apr 8, 2020
Closed
@eddyb eddyb mentioned this pull request Apr 16, 2020
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 18, 2020
@bors
Auto merge of rust-lang#71218 - eddyb:a-lifetime-stranded-in-fn-def, …
52fa23a 
…r=nikomatsakis

outlives: ignore lifetimes shallowly found in `ty::FnDef`s.

Fixes rust-lang#70917 by restoring the pre-rust-lang#70164 behavior for now.

r? @nikomatsakis
@eddyb eddyb mentioned this pull request May 25, 2020
Merged
@varkor varkor mentioned this pull request Jul 22, 2020
Closed
16 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@nikomatsakis nikomatsakis nikomatsakis approved these changes

Assignees

@nikomatsakis nikomatsakis

Labels
F-const_generics `#![feature(const_generics)]` merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: 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.
8 participants
@eddyb @rust-highfive @rust-timer @bors @nikomatsakis @Dylan-DPC-zz @varkor @petrochenkov