Normalize newlines when loading files #62948
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Centril
reviewed
Jul 25, 2019
petrochenkov
added
the
S-waiting-on-review
|
Meta: "draft" PRs don't seem to interact well with our tooling, so I'd rather avoid them. |
|
Two notes:
|
petrochenkov
added
S-waiting-on-author
matklad
force-pushed
the
failable-file-loading
branch
from
b36a450
to
c4ccb42
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
matklad
force-pushed
the
failable-file-loading
branch
2 times, most recently
from
436e45d
to
8e3401a
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
matklad
changed the title
|
Added recovery and precise span reporting. Still on the fence about isolated An alternative would be to just allow then anywhere? In string literals, they will be caught by "escape only char" error, like |
|
I wanted to run crater on this PR once the implementation is ready to find out whether something breaks in practice. |
|
It is ready now |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
matklad
force-pushed
the
failable-file-loading
branch
from
36935a9
to
70d8e84
Compare
petrochenkov
added
S-waiting-on-review
|
☔ The latest upstream changes (presumably #63057) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@matklad |
petrochenkov
added
S-waiting-on-author
bors
added
S-waiting-on-bors
👍 |
bors
added a commit
that referenced
this pull request
Aug 18, 2019
Normalize newlines when loading files Fixes #62865
|
☀️ Test successful - checks-azure |
|
(Just a reminder re. the reference in case you forgot. =P) |
|
The new approach didn't change the definition of the language, it's 100%
behavior preserving refactoring, so the reference is already good.
…On Sun, 18 Aug 2019 at 11:40, Mazdak Farrokhzad ***@***.***> wrote:
(Just a reminder re. the reference in case you forgot. =P)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#62948?email_source=notifications&email_token=AANB3MYXDYHXJL35B33JNZ3QFEDJHA5CNFSM4IGTBHRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4Q3LIA#issuecomment-522302880>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AANB3M63N35HWA5JSLAHFZLQFEDJHANCNFSM4IGTBHRA>
.
|
|
@matklad Oh I see; that's very convenient. 🎉 |
|
The reference actually doesn't discuss CR handling at all. See rust-lang/reference#626. The lexical chapter needs significant updating, but I've been thinking I'll wait for matklad to finish all his changes before tackling it. 😄 I've also been accumulating notes about special cases (like https://internals.rust-lang.org/t/should-we-make-nested-tuple-0-0-legal-syntax/10804/12) that should probably be included. |
|
Speck wise, I think that after #63689 is done, librustc_lexer would be a good candidate for thorough specification (ideally, with a reference declarative impl and a separate test suite). |
I'd like to hear more about your thoughts about this at some point. Recently I have been taking another look at the wg-grammar test strategy, and I think it would be useful to have a conformance test suite that all parsers could use. wg-grammar hasn't been considering the lexical specification at all, but I think it would be natural for that test suite to also cover tokenization. I've been thinking about how to test tokenization since different lexers tokenize differently (the split vs join issue), but haven't gotten too far. |
@ehuss dumped some in rust-lang/wg-grammar#3 (comment) |
matklad
added a commit
to matklad/rust
that referenced
this pull request
Oct 10, 2019
…, r=petrochenkov" This reverts commit ef1ecbe, reversing changes made to fc8765d. That changed unfortunately broke rustfix on windows: rust-lang/rustfix#176 Specifically, what ef1ecbe did was to enforce normalization of \r\n to \n at file loading time, similarly to how we deal with Byte Order Mark. Normalization changes raw offsets in files, which are exposed via `--error-format=json`, and used by rusfix. The proper solution here (which also handles the latent case with BOM) is rust-lang#65074 However, since it's somewhat involved, and we are time sensitive, we prefer to revert the original change on beta.
Mark-Simulacrum
pushed a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Oct 26, 2019
…, r=petrochenkov" This reverts commit ef1ecbe, reversing changes made to fc8765d. That changed unfortunately broke rustfix on windows: rust-lang/rustfix#176 Specifically, what ef1ecbe did was to enforce normalization of \r\n to \n at file loading time, similarly to how we deal with Byte Order Mark. Normalization changes raw offsets in files, which are exposed via `--error-format=json`, and used by rusfix. The proper solution here (which also handles the latent case with BOM) is rust-lang#65074 However, since it's somewhat involved, and we are time sensitive, we prefer to revert the original change on beta.
bors
added a commit
that referenced
this pull request
Oct 26, 2019
[beta] backport rollup This includes a bunch of PRs: * Fix redundant semicolon lint interaction with proc macro attributes #64387 * Upgrade async/await to "used" keywords. #64875 * syntax: fix dropping of attribute on first param of non-method assocated fn #64894 * async/await: improve not-send errors #64895 * Silence unreachable code lint from await desugaring #64930 * Always mark rust and rust-call abi's as unwind #65020 * Account for macro invocation in `let mut $pat` diagnostic. #65123 * Ensure that associated `async fn`s have unique fresh param names #65142 * Add troubleshooting section to PGO chapter in rustc book. #65402 * Upgrade GCC to 8.3.0, glibc to 1.17.0 and crosstool-ng to 1.24.0 for dist-armv7-linux #65302 * Optimize `try_expand_impl_trait_type` #65293 * use precalculated dominators in explain_borrow #65172 * Fix ICE #64964 #64989 * [beta] Revert "Auto merge of #62948 - matklad:failable-file-loading, r=petro… #65273 * save-analysis: Don't ICE when resolving qualified type paths in struct members #65353 * save-analysis: Nest tables when processing impl block definitions #65511 * Avoid ICE when checking `Destination` of `break` inside a closure #65518 * Avoid ICE when adjusting bad self ty #65755 * workaround msys2 bug #65762
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #62865