-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Fix the span for try shorthand expressions #32711
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Thanks! @bors r+ rollup |
📌 Commit 081a7a9 has been approved by |
Needs a test. @bors r- |
@nagisa AFAIK, there is no way to test against regressions of overly long spans. If you know of a way, I'd be happy to write one! |
@marcusklaas I believe you can do something like this: fn main() {
a?; //~ ERROR 2:5: 2:7
} |
as @marcusklaas notes, we don't usually test these, though indeed we probably should... would be nice to add a more targeted way of testing them. I guess that what @jseyfried suggests may actually work. |
It does indeed work. We also have a bunch of regression tests all around already, testing that the span (where it was misaligned before) is correct by using some smart whitespacing in test (though I’m not sure whether the tests actually properly test the span). I don’t see how this case is any different. |
@nagisa these whitespace tests can only verify that some part of the code is part of the span. Here we want to ensure that the span does not contain something. I'll give @jseyfried's approach a try; looks promising! |
Added a test! r? @nagisa |
|
… r=nagisa Fix the span for try shorthand expressions My five character contribution to the rust parser! Fixes rust-lang#32709.
@bors: r- this has a tidy error |
Fixed. Apologies for the mess. |
@bors: r=nagisa 81a37e8d9ba19f0b40509d220da85c7e15e3467d |
http://buildbot.rust-lang.org/builders/auto-linux-musl-64-opt/builds/3787/steps/test/logs/stdio
|
Oh, it still has the rollup flag on! My bad @Manishearth! @bors rollup- r- @marcusklaas need to adjust line numbers from 2 to 16 in the test. |
Ugh - I force pushed an experimental test the last time around :/ My bad. All should be well now. |
Fix the span for try shorthand expressions My five character contribution to the rust parser! Fixes #32709.
💔 Test failed - auto-win-msvc-32-opt |
@bors: retry On Mon, Apr 11, 2016 at 5:13 PM, bors notifications@github.com wrote:
|
Fix the span for try shorthand expressions My five character contribution to the rust parser! Fixes #32709.
My five character contribution to the rust parser! Fixes #32709.