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

don't clone a BacktrackFrame if we are not going to backtrack #5132

Merged
merged 1 commit into from
Mar 6, 2018

Conversation

Eh2406
Copy link
Contributor

@Eh2406 Eh2406 commented Mar 6, 2018

I think this is a fix for #5130.

Currently the only recoverable activate error does not corrupt cx, so this is definitely ok. We should be careful as we add more recoverable activate errors that they don't corrupt cx to the point where the error messages are affected.

@rust-highfive
Copy link

r? @matklad

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

@Eh2406
Copy link
Contributor Author

Eh2406 commented Mar 6, 2018

cc @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+

Nice!

@bors
Copy link
Collaborator

bors commented Mar 6, 2018

📌 Commit 8304b17 has been approved by alexcrichton

bors added a commit that referenced this pull request Mar 6, 2018
don't clone a BacktrackFrame if we are not going to backtrack

I think this is a fix for #5130.

Currently the only recoverable `activate` error does not corrupt `cx`, so this is definitely ok. We should be careful as we add more recoverable `activate` errors that they don't corrupt `cx` to the point where the error messages are affected.
@bors
Copy link
Collaborator

bors commented Mar 6, 2018

⌛ Testing commit 8304b17 with merge c721937...

@Eh2406 Eh2406 mentioned this pull request Mar 6, 2018
@bors
Copy link
Collaborator

bors commented Mar 6, 2018

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

@bors bors merged commit 8304b17 into rust-lang:master Mar 6, 2018
@Eh2406 Eh2406 deleted the less_clones branch March 6, 2018 20:53
@ehuss ehuss added this to the 1.26.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants