Allow writing metadata without llvm#44085
Conversation
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
2a63467 to
dd253c1
Compare
Mark-Simulacrum
left a comment
There was a problem hiding this comment.
I'm happy to see this as a possibility! Left a few minor comments about bootstrap.
src/bootstrap/compile.rs
Outdated
| builder.cargo(compiler, Mode::Libstd, target, "build") | ||
| }else{ | ||
| builder.cargo(compiler, Mode::Libstd, target, "check") | ||
| }; |
There was a problem hiding this comment.
This seems wrong/unrelated? If not, reasoning would be appreciated.
There was a problem hiding this comment.
Should make this no llvm only. When stage 1 rustc doesnt have llvm support it cant build regular rlibs, but only metadata only rlibs.
There was a problem hiding this comment.
Ah. I suppose that makes sense.
src/bootstrap/compile.rs
Outdated
| // missing | ||
| // We also only build the runtimes when --enable-sanitizers (or its | ||
| // config.toml equivalent) is used | ||
| //cargo.env("RUST_FLAGS", "-Zno-trans"); |
There was a problem hiding this comment.
This will need to go before we merge.
src/librustc_driver/driver.rs
Outdated
| ::rustc_trans_utils::find_exported_symbols(tcx, &analysis.reachable); | ||
| let (metadata, _hashes) = | ||
| cstore.encode_metadata(tcx, &link_meta, &exported_symbols); | ||
| let mut builder = Builder::new(File::create("abc.rlib").unwrap()); |
There was a problem hiding this comment.
Forgot to make this write to the file asked to, instead of abc.rlib.
|
|
What is this error message of the "Fix tidy errors" commit: |
|
src/bootstrap/bin/rustc.rs
Outdated
There was a problem hiding this comment.
Didn't you mean !self.build.config.llvm_enabled here?
There was a problem hiding this comment.
This doesnt have self.build.config but i dont think its compiled with the feature, so i think i will have to use an env var.
There was a problem hiding this comment.
Yeah, an env var would be fine. I think there's already precedent for that (i.e. grep for a env var with LLVM in it) so there may not be a need to add a new one.
|
|
☔ The latest upstream changes (presumably #44142) made this pull request unmergeable. Please resolve the merge conflicts. |
bec1f52 to
ea497a2
Compare
|
☔ The latest upstream changes (presumably #43017) made this pull request unmergeable. Please resolve the merge conflicts. |
b88fcf7 to
243d64f
Compare
|
I added a generic |
src/bootstrap/builder.rs
Outdated
src/librustc_driver/driver.rs
Outdated
There was a problem hiding this comment.
Consider moving this outside of the main driver function?
src/librustc_driver/driver.rs
Outdated
There was a problem hiding this comment.
I'm also not sure about the idea of creating dylibs that are actually rlibs.
There was a problem hiding this comment.
Without llvm we cant create real dylibs, but many rustc crates are compiled to dylibs, so by emitting rlibs instead we can compile all rustc libs.
src/librustc_driver/lib.rs
Outdated
There was a problem hiding this comment.
Move this to an out-of-line module?
src/librustc_driver/lib.rs
Outdated
There was a problem hiding this comment.
Move this to an out-of-line module?
src/librustc_trans_traits/lib.rs
Outdated
There was a problem hiding this comment.
Maybe also move the "metadata-only" TransCrate implementation here to rustc_trans_utils?
src/librustc_trans_traits/Cargo.toml
Outdated
There was a problem hiding this comment.
Why does this exist as a separate crate from rustc_trans_utils? They are both depended upon by exactly rustc_trans and rustc_driver, and they semantically perform a very similar task.
There was a problem hiding this comment.
I was thinking about utils to be used by backends only, while traits contains the glue between backends and rustc_driver. I can merge them though.
src/librustc_driver/lib.rs
Outdated
There was a problem hiding this comment.
Can you move all of these items to the TransCrate trait?
There was a problem hiding this comment.
No, because most of them are llvm specific, eg they would still use llvm even if trans is non llvm. (enable_llvm_debug, print_passes)
06155cf to
e130ccc
Compare
|
@arielb1 rebased and fixed most nits |
42e12da to
56c4670
Compare
|
@bors r+ |
|
📌 Commit 3c32c6a has been approved by |
|
⌛ Testing commit 3c32c6a with merge 0e7f2f843dce90acc9e8ec1f29665ba69ac1f844... |
|
💔 Test failed - status-travis |
|
Error should be fixed now. |
|
@bors r+ |
|
📌 Commit 843cd5b has been approved by |
Allow writing metadata without llvm # Todo: * [x] Rebase * [x] Fix eventual errors * [x] <strike>Find some crate to write elf files</strike> (will do it later) Cc #43842
|
☀️ Test successful - status-appveyor, status-travis |
Todo:
Find some crate to write elf files(will do it later)Cc #43842