rustc_metadata: Some reorganization of the module structure #66056

petrochenkov · 2019-11-03T14:56:52Z

The new structure of rustc_metadata (or rather its parts affected by the refactoring) is

├── lib.rs
└── rmeta
    ├── decoder
    │   └── cstore_impl.rs
    ├── decoder.rs
    ├── encoder.rs
    ├── mod.rs
    └── table.rs

(schema is renamed to rmeta.)

The code inside rmeta is pretty self-contained, so we can now privatize almost everything in this module instead of using pub(crate) which was necessary when all these modules accessed their neighbors in the old flat structure.

encoder and decoder work with structures defined by rmeta.
table is a part of rmeta.
cstore_impl actively uses decoder methods and exposes their results through very few public methods (provide and _untracked methods).

r? @eddyb @spastorino

eddyb · 2019-11-03T15:53:27Z

I think I understand what the intent is, but I feel like "schema" stops making sense when it's more than just a bunch of type definitions.

I can think of two ways to improve on the naming (neither that great):

rename librustc_metadata to librustc_cross_crate and schema to metadata
or just rename schema to rmeta (on the basis that it's the "file format name")

I don't have a strong preference for having all the data types in their own module named schema (or if there is a good reason to do so, I've forgotten it), so your structure seems fine.

petrochenkov · 2019-11-03T15:57:12Z

Forgot to cc the related PRs - #66001, #66024, #66028.

bors · 2019-11-04T05:45:24Z

☔ The latest upstream changes (presumably #65838) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2019-11-04T09:09:33Z

@eddyb
I've renamed schema to rmeta and moved it from rmeta.rs to rmeta/mod.rs.

spastorino · 2019-11-04T20:04:13Z

Seems good to me. Leaving the final call to @eddyb

eddyb · 2019-11-05T10:36:50Z

src/librustc_metadata/rmeta/decoder/cstore_impl.rs

@@ -529,6 +528,6 @@ impl CrateStore for cstore::CStore {

    fn metadata_encoding_version(&self) -> &[u8]
    {
-        schema::METADATA_HEADER
+        rmeta::METADATA_HEADER


Heh I just noticed the name of the method is kind of out of sync with what it's actually returning. No need to change it now though.

eddyb · 2019-11-05T10:38:02Z

@bors r+ This is great! 🎉

bors · 2019-11-05T10:38:04Z

📌 Commit 62862c7b33189a3e50c53c4295f13867a09f6bdf has been approved by eddyb

bors · 2019-11-06T05:57:51Z

☔ The latest upstream changes (presumably #66141) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2019-11-06T18:18:22Z

@bors r=eddyb

bors · 2019-11-06T18:18:23Z

📌 Commit 684e4c5a9bf067f7926ec58d1e12c9a471b1e281 has been approved by eddyb

@ghost

Rollup of 5 pull requests Successful merges: - #59789 (Revert two unapproved changes to rustc_typeck.) - #65752 (Use structured suggestions for missing associated items) - #65884 (syntax: ABI-oblivious grammar) - #65974 (A scheme for more macro-matcher friendly pre-expansion gating) - #66017 (Add future incompatibility lint for `array.into_iter()`) Failed merges: - #66056 (rustc_metadata: Some reorganization of the module structure) r? @ghost

bors · 2019-11-07T11:33:43Z

☔ The latest upstream changes (presumably #66180) made this pull request unmergeable. Please resolve the merge conflicts.

And change `rmeta.rs` to `rmeta/mod.rs`

petrochenkov · 2019-11-07T18:20:29Z

@bors r=eddyb

bors · 2019-11-07T18:20:30Z

📌 Commit 5eb1cf1 has been approved by eddyb

@eddyb

rustc_metadata: Some reorganization of the module structure The new structure of `rustc_metadata` (or rather its parts affected by the refactoring) is ``` ├── lib.rs └── rmeta ├── decoder │ └── cstore_impl.rs ├── decoder.rs ├── encoder.rs ├── mod.rs └── table.rs ``` (`schema` is renamed to `rmeta`.) The code inside `rmeta` is pretty self-contained, so we can now privatize almost everything in this module instead of using `pub(crate)` which was necessary when all these modules accessed their neighbors in the old flat structure. `encoder` and `decoder` work with structures defined by `rmeta`. `table` is a part of `rmeta`. `cstore_impl` actively uses decoder methods and exposes their results through very few public methods (`provide` and `_untracked` methods). r? @eddyb @spastorino

@ghost

Rollup of 8 pull requests Successful merges: - #65554 (Enhance the documentation of BufReader for potential data loss) - #65580 (Add `MaybeUninit` methods `uninit_array`, `slice_get_ref`, `slice_get_mut`) - #66049 (consistent handling of missing sysroot spans) - #66056 (rustc_metadata: Some reorganization of the module structure) - #66123 (No more hidden elements) - #66157 (Improve math log documentation examples) - #66165 (Ignore these tests ,since the called commands doesn't exist in VxWorks) - #66190 (rustc_target: inline abi::FloatTy into abi::Primitive.) Failed merges: - #66188 (`MethodSig` -> `FnSig` & Use it in `ItemKind::Fn`) r? @ghost

@eddyb

rustc_metadata: Privatize more things Continuation of rust-lang#66056. The most notable change here is that `CrateMetadata` is moved from `cstore.rs` to `decoder.rs`. Most of uses of `CrateMetadata` fields are in the decoder and uses of `root: CrateRoot` and other fields are so intertwined with each other that it would be hard to move a part of them into `cstore.rs` to privatize `CrateMetadata` fields, so we are going the other way round. `cstore.rs` can probably be dismantled now, but I'll leave this to some other day. Similarly, remaining `CrateMetadata` fields can be privatized by introducing some getter/setter methods, but not today. r? @eddyb

rust-highfive assigned eddyb Nov 3, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 3, 2019

petrochenkov mentioned this pull request Nov 3, 2019

Make CrateMetadata's source_map_import_info and dep_node_index fields private #66001

Closed

petrochenkov force-pushed the metapriv branch 2 times, most recently from 3bbb889 to cf9cadd Compare November 3, 2019 15:49

petrochenkov force-pushed the metapriv branch from cf9cadd to 62862c7 Compare November 4, 2019 09:07

spastorino approved these changes Nov 4, 2019

View reviewed changes

eddyb reviewed Nov 5, 2019

View reviewed changes

eddyb approved these changes Nov 5, 2019

View reviewed changes

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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 5, 2019

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 Nov 6, 2019

petrochenkov force-pushed the metapriv branch from 62862c7 to 684e4c5 Compare November 6, 2019 18:17

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 Nov 6, 2019

Centril mentioned this pull request Nov 7, 2019

Rollup of 5 pull requests #66180

Merged

Centril mentioned this pull request Nov 7, 2019

Rollup of 4 pull requests #66184

Closed

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 Nov 7, 2019

petrochenkov added 6 commits November 7, 2019 21:04

rustc_metadata: Move decoder/encoder/table into mod schema

8590b16

rustc_metadata: Privatize everything in schema and schema/table

8e1ae56

rustc_metadata: Move cstore_impl into mod decoder

0c9d424

rustc_metadata: Privatize everything in decoder

166d5f8

rustc_metadata: Privatize more entities

2b75147

rustc_metadata: Rename schema to rmeta

5eb1cf1

And change `rmeta.rs` to `rmeta/mod.rs`

petrochenkov force-pushed the metapriv branch from 684e4c5 to 5eb1cf1 Compare November 7, 2019 18:20

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 Nov 7, 2019

JohnTitor mentioned this pull request Nov 8, 2019

Rollup of 8 pull requests #66208

Merged

bors merged commit 5eb1cf1 into rust-lang:master Nov 8, 2019

petrochenkov mentioned this pull request Nov 17, 2019

rustc_metadata: Privatize more things #66496

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc_metadata: Some reorganization of the module structure #66056

rustc_metadata: Some reorganization of the module structure #66056

petrochenkov commented Nov 3, 2019 •

edited

Loading

eddyb commented Nov 3, 2019

petrochenkov commented Nov 3, 2019

bors commented Nov 4, 2019

petrochenkov commented Nov 4, 2019

spastorino commented Nov 4, 2019

eddyb Nov 5, 2019

eddyb commented Nov 5, 2019

bors commented Nov 5, 2019

bors commented Nov 6, 2019

petrochenkov commented Nov 6, 2019

bors commented Nov 6, 2019

bors commented Nov 7, 2019

petrochenkov commented Nov 7, 2019

bors commented Nov 7, 2019

rustc_metadata: Some reorganization of the module structure #66056

rustc_metadata: Some reorganization of the module structure #66056

Conversation

petrochenkov commented Nov 3, 2019 • edited Loading

eddyb commented Nov 3, 2019

petrochenkov commented Nov 3, 2019

bors commented Nov 4, 2019

petrochenkov commented Nov 4, 2019

spastorino commented Nov 4, 2019

eddyb Nov 5, 2019

Choose a reason for hiding this comment

eddyb commented Nov 5, 2019

bors commented Nov 5, 2019

bors commented Nov 6, 2019

petrochenkov commented Nov 6, 2019

bors commented Nov 6, 2019

bors commented Nov 7, 2019

petrochenkov commented Nov 7, 2019

bors commented Nov 7, 2019

petrochenkov commented Nov 3, 2019 •

edited

Loading