Cleanup BodyCache #66991

Nashenas88 · 2019-12-03T16:53:19Z

After this PR:

BodyCache is renamed to BodyAndCache
ReadOnlyBodyCache is renamed to ReadOnlyBodyAndCache
ReadOnlyBodyAndCache::body fn is removed and all calls to it are replaced by a deref (possible due to fix of its Deref imp in rustc: split FnAbi's into definitions/direct calls ("of_instance") and indirect calls ("of_fn_ptr"). #65947)

cc @eddyb @oli-obk

rust-highfive · 2019-12-03T16:53:23Z

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

Nashenas88 · 2019-12-03T16:53:29Z

r? @eddyb

Nashenas88 · 2019-12-03T16:54:25Z

This should address @eddyb 's comments here #64736 (comment) (and the comments below in that link).

Nashenas88 · 2019-12-03T16:55:32Z

src/librustc/mir/cache.rs

@@ -220,10 +220,6 @@ impl ReadOnlyBodyCache<'a, 'tcx> {
        self.cache.unwrap_predecessor_locations(loc, self.body)
    }

-    pub fn body(&self) -> &'a Body<'tcx> {


@eddyb Thank you so much for pointing on the &'a in the Deref impl. That was the only reason this fn was left around, and it helped clean up a lot of code. CC @oli-obk

rust-highfive · 2019-12-03T17:13:32Z

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, 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.

2019-12-03T16:57:39.4219675Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-12-03T16:57:40.3051365Z ##[command]git config gc.auto 0
2019-12-03T16:57:40.3058413Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-12-03T16:57:40.3064201Z ##[command]git config --get-all http.proxy
2019-12-03T16:57:40.3069056Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/66991/merge:refs/remotes/pull/66991/merge
---
2019-12-03T17:03:35.2079995Z    Compiling serde_json v1.0.40
2019-12-03T17:03:36.8525170Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2019-12-03T17:03:47.5697799Z     Finished release [optimized] target(s) in 1m 25s
2019-12-03T17:03:47.5796719Z tidy check
2019-12-03T17:03:48.2323409Z tidy error: /checkout/src/librustc_mir/util/def_use.rs:3: line longer than 100 chars
2019-12-03T17:03:48.2767910Z tidy error: /checkout/src/librustc_mir/shim.rs:322: line longer than 100 chars
2019-12-03T17:03:50.2831618Z some tidy checks failed
2019-12-03T17:03:50.2833957Z Found 486 error codes
2019-12-03T17:03:50.2834652Z Found 0 error codes with no tests
2019-12-03T17:03:50.2834751Z Done!
2019-12-03T17:03:50.2834751Z Done!
2019-12-03T17:03:50.2834854Z 
2019-12-03T17:03:50.2839001Z 
2019-12-03T17:03:50.2840118Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2019-12-03T17:03:50.2840554Z 
2019-12-03T17:03:50.2840685Z 
2019-12-03T17:03:50.2849872Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-12-03T17:03:50.2850203Z Build completed unsuccessfully in 0:01:29
2019-12-03T17:03:50.2850203Z Build completed unsuccessfully in 0:01:29
2019-12-03T17:03:50.2906645Z == clock drift check ==
2019-12-03T17:03:50.2918001Z   local time: Tue Dec  3 17:03:50 UTC 2019
2019-12-03T17:03:50.4479048Z   network time: Tue, 03 Dec 2019 17:03:50 GMT
2019-12-03T17:03:50.4482466Z == end clock drift check ==
2019-12-03T17:03:51.7940227Z 
2019-12-03T17:03:51.8012439Z ##[error]Bash exited with code '1'.
2019-12-03T17:03:51.8041518Z ##[section]Starting: Checkout
2019-12-03T17:03:51.8043904Z ==============================================================================
2019-12-03T17:03:51.8043987Z Task         : Get sources
2019-12-03T17:03:51.8044041Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

eddyb · 2019-12-03T17:26:46Z

src/librustc/mir/cache.rs

-impl Deref for ReadOnlyBodyCache<'a, 'tcx> {
-    type Target = Body<'tcx>;
+impl Deref for ReadOnlyBodyAndCache<'a, 'tcx> {
+    type Target = &'a Body<'tcx>;


Btw as I mentioned on the original PR, I've made this change in #65947, so I'd prefer landing that first.

I saw that. I didn't plan to merge this until that one was merged first. I just wanted to make that change in this branch too so I could verify that the other changes wouldn't cause any issues. I'll update the main detail of the PR to make note of that.

bors · 2019-12-04T11:25:00Z

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

Nashenas88 · 2019-12-05T03:23:46Z

@eddyb I rebased the PR and updated the main comment to reflect the minimized changes. Let me know if this still makes sense to move forward, otherwise I can just close this PR.

bors · 2019-12-05T17:49:10Z

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

eddyb

r=me after rebase

…nneeded Index impl, remove body fn rustc_codegen_ssa: Fix BodyAndCache reborrow to Body and change instances of body() call to derefence rustc_mir: Fix BodyAndCache reborrow to Body and change intances of body() call to derefence

oli-obk · 2019-12-06T10:43:25Z

@bors r=eddyb

bors · 2019-12-06T10:43:26Z

📌 Commit a5e144b has been approved by eddyb

@eddyb

Cleanup BodyCache After this PR: - `BodyCache` is renamed to `BodyAndCache` - `ReadOnlyBodyCache` is renamed to `ReadOnlyBodyAndCache` - `ReadOnlyBodyAndCache::body` fn is removed and all calls to it are replaced by a deref (possible due to fix of its `Deref` imp in rust-lang#65947) cc @eddyb @oli-obk

@eddyb

Cleanup BodyCache After this PR: - `BodyCache` is renamed to `BodyAndCache` - `ReadOnlyBodyCache` is renamed to `ReadOnlyBodyAndCache` - `ReadOnlyBodyAndCache::body` fn is removed and all calls to it are replaced by a deref (possible due to fix of its `Deref` imp in rust-lang#65947) cc @eddyb @oli-obk

@eddyb

Cleanup BodyCache After this PR: - `BodyCache` is renamed to `BodyAndCache` - `ReadOnlyBodyCache` is renamed to `ReadOnlyBodyAndCache` - `ReadOnlyBodyAndCache::body` fn is removed and all calls to it are replaced by a deref (possible due to fix of its `Deref` imp in rust-lang#65947) cc @eddyb @oli-obk

@ghost

Rollup of 5 pull requests Successful merges: - #66325 (Change unused_labels from allow to warn) - #66991 (Cleanup BodyCache) - #67101 (use `#[allow(unused_attributes)]` to paper over incr.comp problem) - #67114 (Make `ForeignItem` an alias of `Item`.) - #67129 (Fixes typo) Failed merges: - #66886 (Remove the borrow check::nll submodule) r? @ghost

rust-highfive assigned davidtwco Dec 3, 2019

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

rust-highfive assigned eddyb and unassigned davidtwco Dec 3, 2019

Nashenas88 commented Dec 3, 2019

View reviewed changes

eddyb reviewed Dec 3, 2019

View reviewed changes

Nashenas88 force-pushed the body_cache_cleanup branch from 2325c6c to ebe79b0 Compare December 5, 2019 03:16

eddyb approved these changes Dec 5, 2019

View reviewed changes

Nashenas88 added 3 commits December 5, 2019 22:58

rustc_mir: Fix tidy line lengths

9a21f6e

rustc_codegen_ssa: Fix line accidentally reverted during rebase

a5e144b

Nashenas88 force-pushed the body_cache_cleanup branch from ebe79b0 to a5e144b Compare December 6, 2019 04:16

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 Dec 6, 2019

Centril mentioned this pull request Dec 7, 2019

Rollup of 7 pull requests #67111

Closed

Centril mentioned this pull request Dec 7, 2019

Rollup of 7 pull requests #67132

Closed

Centril mentioned this pull request Dec 8, 2019

Rollup of 5 pull requests #67140

Merged

bors merged commit a5e144b into rust-lang:master Dec 8, 2019

Nashenas88 deleted the body_cache_cleanup branch December 11, 2019 01:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup BodyCache #66991

Cleanup BodyCache #66991

Nashenas88 commented Dec 3, 2019 •

edited

Loading

rust-highfive commented Dec 3, 2019

Nashenas88 commented Dec 3, 2019

Nashenas88 commented Dec 3, 2019

Nashenas88 Dec 3, 2019

rust-highfive commented Dec 3, 2019

eddyb Dec 3, 2019

Nashenas88 Dec 3, 2019

bors commented Dec 4, 2019

Nashenas88 commented Dec 5, 2019

bors commented Dec 5, 2019

eddyb left a comment

oli-obk commented Dec 6, 2019

bors commented Dec 6, 2019

Cleanup BodyCache #66991

Cleanup BodyCache #66991

Conversation

Nashenas88 commented Dec 3, 2019 • edited Loading

rust-highfive commented Dec 3, 2019

Nashenas88 commented Dec 3, 2019

Nashenas88 commented Dec 3, 2019

Nashenas88 Dec 3, 2019

Choose a reason for hiding this comment

rust-highfive commented Dec 3, 2019

eddyb Dec 3, 2019

Choose a reason for hiding this comment

Nashenas88 Dec 3, 2019

Choose a reason for hiding this comment

bors commented Dec 4, 2019

Nashenas88 commented Dec 5, 2019

bors commented Dec 5, 2019

eddyb left a comment

Choose a reason for hiding this comment

oli-obk commented Dec 6, 2019

bors commented Dec 6, 2019

Nashenas88 commented Dec 3, 2019 •

edited

Loading