metadata: Fix zero-normalization of the pos of a MultiByteChar
#24932
Conversation
|
r? @pcwalton (rust_highfive has picked a reviewer for you, use r? to override) |
|
(actually on reflection, even though the original problem case was using non-breaking space, it would probably be a good idea if I used a non-whitespace character in that regression test. let me adjust that.) Update: okay, that is fixed now. |
use visible characters for the multibyte character filler.
pnkfelix
force-pushed
the
fix-issue-24687
branch
from
84c2fd8
to
2ae82fc
Compare
|
\o/ |
|
@bors r+ |
|
📌 Commit 2ae82fc has been approved by |
|
@bors: force The diagnostic one is running now, that's a rather small PR and can wait (probably ought to be rollup) |
|
@bors p=1 |
|
@bors force |
metdata: Fix zero-normalization of the pos of a `MultiByteChar` Fix #24687 The source byte/character mappings for every crate track the collection of multi-characters from its source files specially. When we import the source information for another file into the current compilation unit, we assign its byte-positions unique values by shifting them all by a fixed adjustment, tracked in the `start_pos` field. But when we pull out the source span information for one function from one crate and into our own crate, we need to re-normalize the byte positions: subtracting the old `start_pos` and adding the new `start_pos`. The `new_imported_filemap(..)` method handles adding the new `start_pos`, so all `creader` needs to do is re-normalize each `pos` to zero. It seems like it was indeed trying to do this, but it mistakenly added the old `start_pos` instead of subtracting it.
|
triage: beta-nominated Considering that:
we probably should at least consider cherry-picking it to the beta channel. |
rust-highfive
added
the
beta-nominated
|
For what it’s worth, cherry-picking does not affect Servo as we don’t use the beta or stable channel. |
|
@SimonSapin right, I just meant that if Servo was blocked by this, then it is reasonable to conclude that other clients, who we want to stay on beta/stable channels, will also be blocked by this. |
|
Makes sense. |
|
Also, a quick followup note:
|
Note it is safe, with respect to autobuilds, to land before rust-lang#24945. (In other words, landing this sooner won't break things for anyone any worse than they were already broken, since there are *other* tests that also add `-g` to their flags via `compile-flags: -g`.)
…24687-test Add `-g` (to testcase) that I should have included in PR rust-lang#24932. Note it is safe, with respect to autobuilds, to land before rust-lang#24945. (In other words, landing this sooner won't break things for anyone any worse than they were already broken, since there are *other* tests that also add `-g` to their flags via `compile-flags: -g`.)
|
accepted for backport to beta channel. (as a special case for 1.0; post 1.0 a change like this might need stronger motivation for backport...) |
pnkfelix
added
the
beta-accepted
brson
removed
the
beta-nominated
|
Backported. |
metdata: Fix zero-normalization of the pos of a
MultiByteCharFix #24687
The source byte/character mappings for every crate track the collection of multi-characters from its source files specially. When we import the source information for another file into the current compilation unit, we assign its byte-positions unique values by shifting them all by a fixed adjustment, tracked in the
start_posfield. But when we pull out the source span information for one function from one crate and into our own crate, we need to re-normalize the byte positions: subtracting the oldstart_posand adding the newstart_pos. Thenew_imported_filemap(..)method handles adding the newstart_pos, so allcreaderneeds to do is re-normalize eachposto zero.It seems like it was indeed trying to do this, but it mistakenly added the old
start_posinstead of subtracting it.