Skip to content

dump-mir was causing cycles by invoking item-path-str at bad times - #41777

Merged
bors merged 1 commit into
rust-lang:masterfrom
nikomatsakis:issue-41697-mir-dump-cycle
May 9, 2017
Merged

dump-mir was causing cycles by invoking item-path-str at bad times#41777
bors merged 1 commit into
rust-lang:masterfrom
nikomatsakis:issue-41697-mir-dump-cycle

Conversation

@nikomatsakis

@nikomatsakis nikomatsakis commented May 6, 2017

Copy link
Copy Markdown
Contributor

Workaround for now, but probably a better fix is to opt in to using the types for impls (if we do that at all; maybe filename/line is better).

Fixes #41697

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @pnkfelix

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

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 6, 2017
@arielb1

arielb1 commented May 8, 2017

Copy link
Copy Markdown
Contributor

Can you scope the thread-local just to the call to get_item_path? I think we should get away from using TLS for that because it looks like a recipe for trouble (move all the display impls to free fns, and call the free fns from the display impls).

@nikomatsakis
nikomatsakis force-pushed the issue-41697-mir-dump-cycle branch from 325cf47 to 4f596a0 Compare May 8, 2017 19:45
@nikomatsakis

Copy link
Copy Markdown
Contributor Author

@arielb1 like that?

Workaround for now, but probably a better fix is to opt **in** to using
the types for impls (if we do that at all; maybe filename/line is
better).
@nikomatsakis
nikomatsakis force-pushed the issue-41697-mir-dump-cycle branch from 4f596a0 to cadf187 Compare May 8, 2017 19:59
@arielb1 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-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2017
@arielb1

arielb1 commented May 9, 2017

Copy link
Copy Markdown
Contributor

Yup.
@bors r+

@bors

bors commented May 9, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit cadf187 has been approved by arielb1

@bors

bors commented May 9, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit cadf187 with merge c2d53dc...

bors added a commit that referenced this pull request May 9, 2017
…elb1

dump-mir was causing cycles by invoking item-path-str at bad times

Workaround for now, but probably a better fix is to opt **in** to using the types for impls (if we do that at all; maybe filename/line is better).

Fixes #41697
@bors

bors commented May 9, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing c2d53dc to master...

@bors
bors merged commit cadf187 into rust-lang:master May 9, 2017
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 -Z dump-mir=all

6 participants