remove or encapsulate the remaining non-query data in tcx by nikomatsakis · Pull Request #44501 · rust-lang/rust

nikomatsakis · 2017-09-11T19:00:21Z

I wound up removing the existing cache around inhabitedness since it didn't seem to be adding much value. I reworked const rvalue promotion, but not that much (i.e., I did not split the computation into bits, as @eddyb had tossed out as a suggestion). But it's now demand driven, at least.

cc @michaelwoerister -- see the forbid_reads change in last commit

r? @eddyb -- since the trickiest of this PR is the work on const rvalue promotion

cc #44137

eddyb · 2017-09-11T19:12:11Z

r=me modulo the forbid_reads hack. Also, cc @arielb1

michaelwoerister · 2017-09-11T20:06:51Z

forbid_reads is considered a hack?

eddyb · 2017-09-11T20:15:10Z

@michaelwoerister I meant RegionEraser and how it uses forbid_reads - to me it looks hackier than using a query, and it's only for performance reasons? But I'll defer that to you and/or @arielb1.

bors · 2017-09-11T22:49:54Z

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

Mark-Simulacrum · 2017-09-11T23:12:50Z

Before we merge, let's get some perf numbers - run a try build when ready.

nikomatsakis · 2017-09-11T23:35:48Z

@eddyb I think that example could indeed be a query -- I was considering actually extending the maps macro to support such queries (i.e., queries that are not permitting to read during their code, and hence don't require a task) or just making it be a query. I could go either way.

kennytm · 2017-09-12T09:06:26Z

Missing license.
[00:02:50] tidy error: /checkout/src/librustc/ty/erase_regions.rs: incorrect license

arielb1 · 2017-09-12T11:03:28Z

LGTM

nikomatsakis · 2017-09-12T14:31:58Z

@bors r=eddyb

bors · 2017-09-12T14:32:00Z

📌 Commit 49a2c08 has been approved by eddyb

bors · 2017-09-13T08:24:18Z

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

alexcrichton · 2017-09-15T18:13:37Z

@bors r=eddyb

bors · 2017-09-15T18:13:37Z

📌 Commit b952f47 has been approved by eddyb

bors · 2017-09-17T16:29:43Z

⌛ Testing commit b952f477c63226e1059c1c4868db962ed10d93d9 with merge 3e4fe6c4557ff9caadba4f43ba263716483cce26...

bors · 2017-09-17T16:52:11Z

💔 Test failed - status-travis

kennytm · 2017-09-18T08:21:45Z

Could not compile stage1-std in x86_64-gnu-incremental with a BorrowMutError somewhere.

[00:18:53] error: internal compiler error: unexpected panic
[00:18:53] [00:18:53] note: the compiler unexpectedly panicked. this is a bug.
[00:18:53] [00:18:53] note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
[00:18:53] [00:18:53] note: rustc 1.22.0-dev running on x86_64-unknown-linux-gnu
[00:18:53] [00:18:53] thread 'rustc' panicked at 'already borrowed: BorrowMutError', /checkout/src/libcore/result.rs:906:4
[00:18:53] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:18:53] [00:18:53] error: Could not compile `std`.

alexcrichton · 2017-09-18T15:30:58Z

@bors: r-

reappeared in the travis queue

bors · 2017-09-18T23:49:45Z

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

nikomatsakis · 2017-09-19T19:42:31Z

@bors r=eddyb

bors · 2017-09-19T19:42:32Z

📌 Commit 10069e5 has been approved by eddyb

nikomatsakis · 2017-10-16T21:36:51Z

cc @Manishearth @llogiq @mcarton @oli-obk -- apparently I've broken clippy. Looks like primarily this is a matter of now needing to supply the rvalue_promotable_map to the ExprUseVisitor, but also the removal of the rvalue_promotable_to_static map (replaced by the rvalue_promotable query, example edit).

(Are there some directions for the steps to prepare a PR to clippy to fix this stuff?)

oli-obk · 2017-10-17T06:11:26Z

(Are there some directions for the steps to prepare a PR to clippy to fix this stuff?)

Directions can be found at https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies

Generally, you can just fix clippy by compiling it via ./x.py build src/tools/clippy and open a PR once it builds. We'll take care to reenable clippy once your changes hit master and are merged into clippy master.

Remember to set submodules = false in config.toml, otherwise the next call to x.py will erase your submodule changes

If you consider the PR to clippy to be too involved (e.g. because we used an API so wrongly it'll take a lot of effort to fix our usage), we'll fix it ourselvels.

nikomatsakis · 2017-10-17T15:30:33Z

@arielb1

What can erase_regions depend on?

Actually, looking again, I'm not sure. I thought that I had seen -- or maybe just imagined -- some path in anonymize_late_bound_regions that accesses one of the adt-defs or something, but looking again I don't see any such path. Perhaps there was some other bug in forbid_reads the way I wrote it? I wish I could reproduce the problem.

Regardless, as @eddyb pointed out earlier, a regular query is simpler, so that may have been a bit of premature perf. optimization on my part. I think my ideal preference would be to keep it as a query, but add an option to declare a "non-read" query that skips dependency tracking (using something like forbid_reads to check for correctness).

eddyb · 2017-10-17T15:43:13Z

@bors r+

bors · 2017-10-17T15:43:14Z

📌 Commit 7715f97 has been approved by eddyb

bors · 2017-10-17T20:22:38Z

⌛ Testing commit 7715f97 with merge 76e77ec828027f688faaa615fabad5d641223a76...

bors · 2017-10-18T02:42:08Z

💔 Test failed - status-travis

kennytm · 2017-10-18T07:28:39Z

@bors retry

macOS... spuriously canceled after 6 hours 😂

bors · 2017-10-18T10:44:15Z

⌛ Testing commit 7715f97 with merge 7a4f394...

@eddyb

…, r=eddyb remove or encapsulate the remaining non-query data in tcx I wound up removing the existing cache around inhabitedness since it didn't seem to be adding much value. I reworked const rvalue promotion, but not that much (i.e., I did not split the computation into bits, as @eddyb had tossed out as a suggestion). But it's now demand driven, at least. cc @michaelwoerister -- see the `forbid_reads` change in last commit r? @eddyb -- since the trickiest of this PR is the work on const rvalue promotion cc #44137

bors · 2017-10-18T13:36:12Z

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 7a4f394 to master...

oli-obk · 2017-10-20T07:00:20Z

Would it make sense to simplify the map returned by rvalue_promotable_map to a set?

@nikomatsakis

Use a `Set<T>` instead of a `Map<T, bool>` r? @nikomatsakis introduced in #44501

rust-highfive assigned eddyb Sep 11, 2017

nikomatsakis mentioned this pull request Sep 11, 2017
incremental: refactor out non-query data from tcx #44137
Closed
13 tasks

nikomatsakis force-pushed the issue-44137-non-query-data-in-tcx branch from c58e725 to fb2e369Compare September 12, 2017 08:39

kennytm reviewed Sep 12, 2017
View reviewed changes

arielb1 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 12, 2017

nikomatsakis mentioned this pull request Sep 14, 2017
inhabitedness cache handles cycles incorrectly #44402
Closed

alexcrichton force-pushed the issue-44137-non-query-data-in-tcx branch from 49a2c08 to b952f47Compare September 15, 2017 18:13

nikomatsakis force-pushed the issue-44137-non-query-data-in-tcx branch from b952f47 to 10069e5Compare September 19, 2017 19:41

arielb1 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 Sep 19, 2017

nikomatsakis added 2 commits October 16, 2017 17:32

fix nits
165cbce

bump up the recursion limit
0ae18a1

nikomatsakis force-pushed the issue-44137-non-query-data-in-tcx branch from 94449a5 to dd3a4f4Compare October 16, 2017 21:33

kennytm 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 Oct 17, 2017

oli-obk mentioned this pull request Oct 17, 2017
Validate miri against the HIR const evaluator #45002
Merged

nikomatsakis added 2 commits October 17, 2017 11:24

make erase_regions use a query instead of an ad-hoc cache
0e67d17

break clippy
7715f97

kennytm 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 17, 2017

nikomatsakis force-pushed the issue-44137-non-query-data-in-tcx branch from a673c3b to 7715f97Compare October 17, 2017 15:56

bors merged commit 7715f97 into rust-lang:masterOct 18, 2017

bors mentioned this pull request Oct 18, 2017
Create NormalizeTy query #44984
Merged

dwrensha added a commit to dwrensha/seer that referenced this pull request Oct 19, 2017
rustup forrust-lang/rust#44501
d264d90

oli-obk mentioned this pull request Nov 3, 2017
Use a Set<T> instead of a Map<T, bool>#45736
Merged

bors added a commit that referenced this pull request Nov 9, 2017
Auto merge of#45736- oli-obk:rvalue_promotable_map, r=nikomatsakis
fd9ecfd
Use a `Set<T>` instead of a `Map<T, bool>` r? @nikomatsakis introduced in #44501

kennytm mentioned this pull request Apr 5, 2018
ci: Remove x86_64-gnu-incremental builder #49674
Merged

Uh oh!

Uh oh!

Conversation

nikomatsakis commented Sep 11, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eddyb commented Sep 11, 2017

Uh oh!

michaelwoerister commented Sep 11, 2017

Uh oh!

eddyb commented Sep 11, 2017

Uh oh!

bors commented Sep 11, 2017

Uh oh!

Mark-Simulacrum commented Sep 11, 2017

Uh oh!

nikomatsakis commented Sep 11, 2017

Uh oh!

kennytmSep 12, 2017

Choose a reason for hiding this comment

Uh oh!

arielb1 commented Sep 12, 2017

Uh oh!

nikomatsakis commented Sep 12, 2017

Uh oh!

bors commented Sep 12, 2017

Uh oh!

bors commented Sep 13, 2017

Uh oh!

alexcrichton commented Sep 15, 2017

Uh oh!

bors commented Sep 15, 2017

Uh oh!

bors commented Sep 17, 2017

Uh oh!

bors commented Sep 17, 2017

Uh oh!

kennytm commented Sep 18, 2017

Uh oh!

alexcrichton commented Sep 18, 2017

Uh oh!

bors commented Sep 18, 2017

Uh oh!

nikomatsakis commented Sep 19, 2017

Uh oh!

bors commented Sep 19, 2017

Uh oh!

nikomatsakis commented Oct 16, 2017

Uh oh!

oli-obk commented Oct 17, 2017

Uh oh!

nikomatsakis commented Oct 17, 2017

Uh oh!

eddyb commented Oct 17, 2017

Uh oh!

bors commented Oct 17, 2017

Uh oh!

bors commented Oct 17, 2017

Uh oh!

bors commented Oct 18, 2017

Uh oh!

kennytm commented Oct 18, 2017

Uh oh!

bors commented Oct 18, 2017

Uh oh!

bors commented Oct 18, 2017

Uh oh!

oli-obk commented Oct 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

nikomatsakis commented Sep 11, 2017•
edited
Loading