-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Various fixes in macro code. #1606
Conversation
This code should't have any portability problems, but just in case can you please test it on the build bots before merging. Thanks. |
This fixes several issues but doesn't appear to include regression tests. Can you add some compile-fail tests for the two issues this closes? They will include probably an |
Note: Please don't accept until I give the all clear. Want to rebase now after some more testing... |
Need a better fix, right now it is just causing even more confusion, for example in issue rust-lang#1448 and rust-lang#1387. This reverts commit 1e4de33.
This correctly fixes issue rust-lang#1362. chpos/byte_pos are now the offsets within a particular file, but rather the offsets within a virtual file with is formed by combing all of the modules within a crate. Thus, resetting them to 0 causes an overlap and hence, bogus source locations. Fix rust-lang#1362 by moving chpos/byte_pos to parse_sess so that new_parser_from_source_str has access to them and hence can chose an initial value that is not already been used in the crate. Note that the trigger for bug 1361 was that syntax/ext/expand.rs calls parse_expr_from_source_str (which calls new_parser_from_source_str) using the same codemap as the current crate (and hence causing overlap with files in the crate as new_parser_from_source_str resets the chpos/byte_pos to 0).
This involved changing the prototype for the callbacks to thread the span though. A wrapper function, fold::wrap, can be used to wrap the old style callbacks.
the replacement and not the span of the pattern variable. Fixes issue rust-lang#1448, and rust-lang#1387.
Okay, ready to be accepted. I fixed the bug brson pointed out on IRC and it looks like it passed the build bot build, but just in case please try it on the build-bots again accepting. |
Still ready to go, just rebased to fix an incorrect issue reference in the commit message. |
Although its not really needed. Without that fix, reported spans will likely be bogus if the error is within the first couple of lines (probable around 5) of that file. Thus, many of the compile-fail tests will fail due to incorrect location.
This looks good to me. Pushing to try one last time. |
It's in! |
Correctly fixes Issue #1362. Fixes #1387 and #1448.
See the bug reports and commit messages for details.
Note my work on a #rust as discussed with Graydon will depend on these commits.