Skip to content

[breaking-batch] Move more uses of panictry! out of libsyntax#31631

Merged
bors merged 2 commits into
rust-lang:masterfrom
jonas-schievink:agoraphobia
Mar 9, 2016
Merged

[breaking-batch] Move more uses of panictry! out of libsyntax#31631
bors merged 2 commits into
rust-lang:masterfrom
jonas-schievink:agoraphobia

Conversation

@jonas-schievink
Copy link
Copy Markdown
Contributor

@jonas-schievink jonas-schievink commented Feb 13, 2016

No description provided.

@rust-highfive
Copy link
Copy Markdown
Contributor

rust-highfive commented Feb 13, 2016

r? @sfackler

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

@jonas-schievink
Copy link
Copy Markdown
Contributor Author

jonas-schievink commented Feb 13, 2016

...I messed that up. Fixing!

@alexcrichton
Copy link
Copy Markdown
Member

alexcrichton commented Feb 13, 2016

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned sfackler Feb 13, 2016
@jonas-schievink
Copy link
Copy Markdown
Contributor Author

jonas-schievink commented Feb 13, 2016

This should work. I'm not too happy about panictry! infecting even more crates, but since the Err case propagates a DiagnosticBuilder which causes side effects (or must cause side effects, otherwise it panics), I'm not sure how to handle the PResult otherwise.

@bors
Copy link
Copy Markdown
Collaborator

bors commented Feb 15, 2016

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

@jonas-schievink
Move more uses of panictry! out of libsyntax
3a87278 
[breaking-change] for syntax extensions
@jonas-schievink
Copy link
Copy Markdown
Contributor Author

jonas-schievink commented Feb 16, 2016

Rebased

nrc
nrc reviewed Feb 19, 2016
View reviewed changes
Comment thread src/librustc_driver/driver.rs Outdated
let (outputs, trans) = {
let (outputs, expanded_crate, id) = {
let krate = phase_1_parse_input(sess, cfg, input);
let krate = panictry!(phase_1_parse_input(sess, cfg, input));
Copy link
Copy Markdown
Member

@nrc nrc Feb 19, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't panic here, we should return an error instead

@nrc
Copy link
Copy Markdown
Member

nrc commented Feb 19, 2016

r+ with the above changes (sorry the review took so long).

@nrc
Copy link
Copy Markdown
Member

nrc commented Mar 9, 2016

@bors: r+

@bors
Copy link
Copy Markdown
Collaborator

bors commented Mar 9, 2016

📌 Commit 11e0ba4 has been approved by nrc

@bors
Copy link
Copy Markdown
Collaborator

bors commented Mar 9, 2016

⌛ Testing commit 11e0ba4 with merge cbbd3d9...

bors added a commit that referenced this pull request Mar 9, 2016
@bors
Auto merge of #31631 - jonas-schievink:agoraphobia, r=nrc
cbbd3d9 
[breaking-batch] Move more uses of `panictry!` out of libsyntax
@bors
Copy link
Copy Markdown
Collaborator

bors commented Mar 9, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

@bors bors merged commit 11e0ba4 into rust-lang:master Mar 9, 2016
@jonas-schievink jonas-schievink deleted the agoraphobia branch March 9, 2016 15:15
@jonas-schievink
Copy link
Copy Markdown
Contributor Author

jonas-schievink commented Mar 9, 2016

Ah, dang it, I totally forgot about #31645, now this'll break stuff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nrc nrc

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@jonas-schievink @rust-highfive @alexcrichton @bors @nrc @sfackler