Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
May 9, 2017

Conversation

nikomatsakis
Copy link
Contributor

@nikomatsakis nikomatsakis commented May 6, 2017

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
Collaborator

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
Copy link
Contributor

arielb1 commented May 8, 2017

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
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
Copy link
Contributor

arielb1 commented May 9, 2017

Yup.
@bors r+

@bors
Copy link
Contributor

bors commented May 9, 2017

📌 Commit cadf187 has been approved by arielb1

@bors
Copy link
Contributor

bors commented May 9, 2017

⌛ 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
Copy link
Contributor

bors commented May 9, 2017

☀️ 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