Store idents for `DefPathData` into crate metadata #70077

Aaron1011 · 2020-03-17T16:08:14Z

Previously, we threw away the Span associated with a definition's
identifier when we encoded crate metadata, causing us to lose location
and hygiene information.

We now store the identifier's Span in a side table, which gets encoded
into the crate metadata. When we decode items from the metadata, we
combine the name and span back into an Ident.

This improves the output of several tests, which previously had messages
suppressed due to dummy spans.

This is a prerequisite for #68686, since throwing away a Span means
that we lose hygiene information.

rust-highfive · 2020-03-17T16:08:17Z

r? @eddyb

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

src/librustc_metadata/rmeta/encoder.rs

src/librustc_metadata/rmeta/decoder.rs

Aaron1011 · 2020-03-18T21:11:37Z

@petrochenkov: Did you want to do a perf run for this?

petrochenkov · 2020-03-19T08:33:01Z

@Aaron1011
Yes, it would be good to run perf on this PR before merging it.

src/librustc_metadata/rmeta/decoder.rs

src/librustc_metadata/rmeta/encoder.rs

src/librustc_metadata/rmeta/decoder.rs

Aaron1011 · 2020-03-19T19:28:47Z

@petrochenkov: I've addressed your comments.

src/librustc_metadata/rmeta/encoder.rs

petrochenkov · 2020-03-19T22:08:52Z

An accidental submodule update breaks CI.

Aaron1011 · 2020-03-20T03:14:41Z

@petrochenkov: Updated. If this looks good to you, I'll squash the commits.

petrochenkov · 2020-03-20T09:09:10Z

@bors try @rust-timer queue

…t, r=petrochenkov Store idents for `DefPathData` into crate metadata Previously, we threw away the `Span` associated with a definition's identifier when we encoded crate metadata, causing us to lose location and hygiene information. We now store the identifier's `Span` in a side table, which gets encoded into the crate metadata. When we decode items from the metadata, we combine the name and span back into an `Ident`. This improves the output of several tests, which previously had messages suppressed due to dummy spans. This is a prerequisite for rust-lang#68686, since throwing away a `Span` means that we lose hygiene information.

Centril · 2020-03-23T03:14:41Z

Failed in #70288 (comment), @bors r-

Previously, we threw away the `Span` associated with a definition's identifier when we encoded crate metadata, causing us to lose location and hygiene information. We now store the identifier's `Span` in the crate metadata. When we decode items from the metadata, we combine the name and span back into an `Ident`. This improves the output of several tests, which previously had messages suppressed due to dummy spans. This is a prerequisite for rust-lang#68686, since throwing away a `Span` means that we lose hygiene information.

Aaron1011 · 2020-03-23T05:02:16Z

This appears to be another instance of #53081: apparently, there are platform-specific issues with printing sysroot spans, which is causing some platforms to not print the output expected by the tests.

Aaron1011 · 2020-03-23T06:06:55Z

@petrochenkov: I've ignored the failing tests on the relevant platforms. It's unfortunate that this is required, but I'd prefer not to block this PR on resolving #53081.

petrochenkov · 2020-03-23T08:06:35Z

@bors r+

bors · 2020-03-23T08:06:36Z

📌 Commit 86b8dea has been approved by petrochenkov

…t, r=petrochenkov Store idents for `DefPathData` into crate metadata Previously, we threw away the `Span` associated with a definition's identifier when we encoded crate metadata, causing us to lose location and hygiene information. We now store the identifier's `Span` in a side table, which gets encoded into the crate metadata. When we decode items from the metadata, we combine the name and span back into an `Ident`. This improves the output of several tests, which previously had messages suppressed due to dummy spans. This is a prerequisite for rust-lang#68686, since throwing away a `Span` means that we lose hygiene information.

@ghost

Rollup of 8 pull requests Successful merges: - rust-lang#68884 (Make the `type_of` return a generic type for generators) - rust-lang#69788 (Fix sequence of Type and Trait in optin-builtin-traits in Unstable Book) - rust-lang#70074 (Expand: nix all fatal errors) - rust-lang#70077 (Store idents for `DefPathData` into crate metadata) - rust-lang#70213 (traits/fulfill: allow `stalled_on` to track `ty::Const::Infer(_)` (unused yet).) - rust-lang#70259 (Use Reveal::All in MIR optimizations) - rust-lang#70284 (correctly handle const params in type_of) - rust-lang#70289 (Refactor `codegen`) Failed merges: r? @ghost

rust-highfive assigned eddyb Mar 17, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 17, 2020

Aaron1011 mentioned this pull request Mar 17, 2020

Store Ident in DefPathData instead of Symbol #69906

Closed

petrochenkov self-assigned this Mar 17, 2020

petrochenkov reviewed Mar 18, 2020

View reviewed changes

src/librustc_metadata/rmeta/encoder.rs Outdated Show resolved Hide resolved

src/librustc_metadata/rmeta/decoder.rs Outdated Show resolved Hide resolved

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 18, 2020

Aaron1011 changed the title ~~Store idents for DefPathData in a side table~~ Store idents for DefPathData into crate metadata Mar 18, 2020

This comment has been minimized.

Sign in to view

Aaron1011 force-pushed the feature/new-def-path-ident branch from 50d5890 to 3760e40 Compare March 19, 2020 04:01

petrochenkov reviewed Mar 19, 2020

View reviewed changes

eddyb reviewed Mar 19, 2020

View reviewed changes

src/librustc_metadata/rmeta/decoder.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

Aaron1011 force-pushed the feature/new-def-path-ident branch from 3760e40 to b442340 Compare March 19, 2020 19:01

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 19, 2020

This comment has been minimized.

Sign in to view

petrochenkov reviewed Mar 19, 2020

View reviewed changes

src/librustc_metadata/rmeta/encoder.rs Outdated Show resolved Hide resolved

src/librustc_metadata/rmeta/encoder.rs Outdated Show resolved Hide resolved

src/librustc_metadata/rmeta/encoder.rs Outdated Show resolved Hide resolved

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 19, 2020

Aaron1011 force-pushed the feature/new-def-path-ident branch from e7d4cae to 35fb1cb Compare March 20, 2020 01:55

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2020

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 20, 2020

Dylan-DPC-zz mentioned this pull request Mar 22, 2020

Rollup of 6 pull requests #70288

Closed

Aaron1011 mentioned this pull request Mar 23, 2020

Tracking issue for RFC 2091: Implicit caller location #47809

Closed

3 tasks

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 23, 2020

Ignore tests on some platforms due to rust-lang#53081

86b8dea

Aaron1011 force-pushed the feature/new-def-path-ident branch from 1f13a4f to 86b8dea Compare March 23, 2020 06:05

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 23, 2020

Centril mentioned this pull request Mar 24, 2020

Rollup of 7 pull requests #70347

Closed

Centril mentioned this pull request Mar 24, 2020

Rollup of 8 pull requests #70348

Closed

Centril mentioned this pull request Mar 24, 2020

Rollup of 8 pull requests #70351

Merged

bors merged commit d626f5b into rust-lang:master Mar 24, 2020

Aaron1011 deleted the feature/new-def-path-ident branch March 24, 2020 14:50

Store idents for DefPathData into crate metadata #70077

Store idents for DefPathData into crate metadata #70077

Uh oh!

Conversation

Aaron1011 commented Mar 17, 2020

Uh oh!

rust-highfive commented Mar 17, 2020

Uh oh!

Uh oh!

Uh oh!

Aaron1011 commented Mar 18, 2020

Uh oh!

This comment has been minimized.

petrochenkov commented Mar 19, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Aaron1011 commented Mar 19, 2020

Uh oh!

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

petrochenkov commented Mar 19, 2020

Uh oh!

Aaron1011 commented Mar 20, 2020

Uh oh!

petrochenkov commented Mar 20, 2020

Uh oh!

Centril commented Mar 23, 2020

Uh oh!

Aaron1011 commented Mar 23, 2020

Uh oh!

Aaron1011 commented Mar 23, 2020

Uh oh!

petrochenkov commented Mar 23, 2020

Uh oh!

bors commented Mar 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Store idents for `DefPathData` into crate metadata #70077

Store idents for `DefPathData` into crate metadata #70077