metadata: Fix zero-normalization of the pos of a MultiByteChar

[pnkfelix]

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.

[rust-highfive]

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

[pnkfelix]

(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.

[Manishearth]

\o/

cc @larsbergstrom

[huonw]

@bors r+

[bors]

📌 Commit 2ae82fc has been approved by huonw

[Manishearth]

@bors: force

The diagnostic one is running now, that's a rather small PR and can wait (probably ought to be rollup)

[dotdash]

@bors p=1

[dotdash]

@bors force

[bors]

⌛ Testing commit 2ae82fc with merge 551a74d...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

[pnkfelix]

triage: beta-nominated

Considering that:

• this was important for servo,
• it fixes an ICE
• it is pretty low-risk

we probably should at least consider cherry-picking it to the beta channel.

[SimonSapin]

For what it’s worth, cherry-picking does not affect Servo as we don’t use the beta or stable channel.

[pnkfelix]

@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.

[SimonSapin]

Makes sense.

[pnkfelix]

Also, a quick followup note:

• The regression test on this really should have had "compiler-flags: -g" or whatever the directive is for embedded those options in the tests.
• As it is written in this commit, it is actually I think unlikely to test much of anything, because we do not have any builders to my knowledge that do --enable-debug or a blanket -g on the test suite.
• So why didn't I run it that way? Because of rustc fails if passed -g -g; make check breaks under --enable-debug. #24937

[pnkfelix]

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...)

[brson]

Backported.