Uh oh!
There was an error while loading. Please reload this page.
Improve error handling in libsyntax - #29285
Conversation
rust-highfive
commented
Oct 24, 2015
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
e8fe2ac to
113ee6fCompareThere was a problem hiding this comment.
Do we need to panic in these places? We're already declaring a fatal exception, what happens if we just carry on? Don't we exit any way?
There was a problem hiding this comment.
Oh, you're changing fatal errors to not panic. That seems maybe not a good idea? Could you use non-fatal errors in libsyntax instead? Or maybe it is a good idea since it makes the panicking more explicit.
There was a problem hiding this comment.
Well, on master, we're in the slightly odd situation where fatal() panics, but span_fatal() doesn't panic; it probably doesn't make sense for it to stay that way.
In terms of how this should work long-term... libsyntax has basically settled on a "don't panic on fatal errors" approach, so it doesn't really make sense for libsyntax to provide a panicking fatal() method. On the other hand, libsyntax also provides the canonical implementation of fatal errors... so maybe fatal errors should panic, but libsyntax just shouldn't use the fatal() functions.
There was a problem hiding this comment.
OK, this sounds reasonable, thanks for the explanantion
nrc
commented
Oct 26, 2015
r+, but has some test fails which need fixing. |
113ee6f to
77f186aCompareeefriedman
commented
Oct 28, 2015
@nrc Updated; 77f186a fixes the test failure. |
nrc
commented
Oct 28, 2015
@eefriedman still some tests failing |
Rename parse_* to parse_*_panic, and add parse_attribute_panic.
77f186a to
e7d3ae6Compareeefriedman
commented
Oct 29, 2015
Tests fixed in e7d3ae6. |
eefriedman
commented
Oct 30, 2015
@nrc Ping. |
nrc
commented
Nov 3, 2015
@bors: r+ (sorry @eefriedman I've been travelling) |
bors
commented
Nov 3, 2015
📌 Commit e7d3ae6 has been approved by |
A set of commits which pushes some panics out of core parser methods, and into users of those parser methods.
bors
commented
Nov 3, 2015
bors
commented
Nov 3, 2015
rust-lang/rust#29582 renamed `TtToken` to `Token`. rust-lang/rust#29285 renamed `parse_expr` to `parse_expr_panic`.
A set of commits which pushes some panics out of core parser methods, and into users of those parser methods.