Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). #33602
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
|
r? @jroesch (rust_highfive has picked a reviewer for you, use r? to override) |
|
I'm going to review this right now.. |
|
📌 Commit adee551 has been approved by |
|
@bors r=michaelwoerister d'oh this PR wasn't even in the rollup 😫 |
|
📌 Commit adee551 has been approved by |
eddyb
added a commit
to eddyb/rust
that referenced
this pull request
May 13, 2016
…rister Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). Removes the item symbol map in metadata, as we can now generate them in a deterministic manner. The `-Z no-trans` change lets the LLVM passes and linking run, but with just metadata and no code. It fails while trying to link a binary because there's no `main` function, which is correct but not good UX. There's also no way to easily throw away all of the artifacts to rebuild with actual code generation. We might want `cargo check` to do that using cargo-internal information and then it would just work. cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister
|
@bors rollup- |
|
☔ The latest upstream changes (presumably #33632) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@bors r=michaelwoerister |
|
📌 Commit 5087556 has been approved by |
|
Oh wait I didn't fix the failure in @alexcrichton's test. @bors r- |
|
@bors r=michaelwoerister |
|
📌 Commit d2ded31 has been approved by |
|
⌛ Testing commit d2ded31 with merge 33a8175... |
|
💔 Test failed - auto-linux-64-opt-rustbuild |
|
@bors r=michaelwoerister |
|
📌 Commit a619901 has been approved by |
bors
added a commit
that referenced
this pull request
May 25, 2016
Save metadata even with -Z no-trans (e.g. for multi-crate cargo check). Removes the item symbol map in metadata, as we can now generate them in a deterministic manner. The `-Z no-trans` change lets the LLVM passes and linking run, but with just metadata and no code. It fails while trying to link a binary because there's no `main` function, which is correct but not good UX. There's also no way to easily throw away all of the artifacts to rebuild with actual code generation. We might want `cargo check` to do that using cargo-internal information and then it would just work. cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister
bors
added a commit
that referenced
this pull request
Jun 22, 2016
…haelwoerister Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools
bors
added a commit
that referenced
this pull request
Jun 28, 2016
Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools
bors
added a commit
that referenced
this pull request
Jul 8, 2016
Drive trans from the output of the translation item collector This PR changes the way how translation works above the item level. Instead of walking the HIR and calling `trans_item()` on everything encountered (while instantiating monomorphizations on-demand), we now just process the list of translation items generated by the `trans::collector`. Using the collector has the benefit of being able to know the exact set of monomorphizations and symbols before actually running translation, something that is crucial for incremental compilation (but also has [other benefits](#33602)). The collector has existed for quite a while now, but so far it's output was only used for running some auto-tests. With this PR it becomes the only source of truth about what gets translated. One modification we had to make, compared to the initial approach, is that closures are not represented as their own `TransItems`. Doing so, while still supporting non-MIR-based translation, would have been prohibitively complex, and not worth the trouble since legacy-trans will disappear sooner or later. Once there is solely MIR-trans, it would be a good idea to make closures `TransItems` again. This PR removes the most obvious functions and tables that are not needed anymore, but there's definitely still more cleanup possible later on (e.g. `monomorphize::monomorphic_fn()` does very little at this point). Since there are already more than 10 commits in here, doing this in a separate PR seems to be a better idea. These changes definitely warrant a crater run. Thanks @Aatch, for taking on one of the more tedious tasks during the dev-sprint! Thanks @eddyb, for doing some nice refactorings to symbol name generation and making sure these landed so I could use them! cc @rust-lang/compiler cc @rust-lang/tools
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.
Removes the item symbol map in metadata, as we can now generate them in a deterministic manner.
The
-Z no-transchange lets the LLVM passes and linking run, but with just metadata and no code.It fails while trying to link a binary because there's no
mainfunction, which is correct but not good UX.There's also no way to easily throw away all of the artifacts to rebuild with actual code generation.
We might want
cargo checkto do that using cargo-internal information and then it would just work.cc @alexcrichton @nikomatsakis @Aatch @michaelwoerister