Reduce special casing for the panic runtime #140809
Conversation
|
rustbot has assigned @compiler-errors. Use |
rustbot
added
S-waiting-on-review
|
Some changes occurred in compiler/rustc_codegen_ssa |
|
I was looking at making panic_abort and panic_unwind depend on libstd (#139103 (comment)). These changes make this easier, though is still blocked on the fact that when compiling libstd as dylib, panic_unwind already needs to be present. |
This comment has been minimized.
This comment has been minimized.
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
6d4613c to
c7e8663
Compare
|
The Miri subtree was changed cc @rust-lang/miri |
This comment has been minimized.
This comment has been minimized.
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
c7e8663 to
93cf365
Compare
|
☔ The latest upstream changes (presumably #140176) made this pull request unmergeable. Please resolve the merge conflicts. |
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
93cf365 to
a9d5498
Compare
|
☔ The latest upstream changes (presumably #141024) made this pull request unmergeable. Please resolve the merge conflicts. |
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
d730005 to
7d0f158
Compare
|
☔ The latest upstream changes (presumably #141238) made this pull request unmergeable. Please resolve the merge conflicts. |
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
7d0f158 to
0dfacc7
Compare
|
☔ The latest upstream changes (presumably #141730) made this pull request unmergeable. Please resolve the merge conflicts. |
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
0dfacc7 to
6df5456
Compare
|
r? wesleywiser |
|
Compiler changes look good to me but I'd like someone from libs to review the change in exposed standard library features to make sure there's no concerns there. r? libs |
|
Looks fine to me. @bors r=wesleywiser,ibraheemdev |
bors
removed
the
S-waiting-on-review
bors
added
S-waiting-on-review
|
I think LTO is causing the |
|
@bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2 |
Reduce special casing for the panic runtime See the individual commits for more info. try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
This comment has been minimized.
This comment has been minimized.
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
6e801de to
2489701
Compare
|
@bors2 try cancel |
|
Try build cancelled. Cancelled workflows: |
Reduce special casing for the panic runtime See the individual commits for more info. try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
|
💔 Test failed
|
|
This PR modifies cc @jieyouxu |
|
Updated the test that unstably includes the panic_unwind panic runtime without libstd to use @bors2 try jobs=x86_64-msvc-1,x86_64-msvc-2 |
Reduce special casing for the panic runtime See the individual commits for more info. try-job: x86_64-msvc-1 try-job: x86_64-msvc-2
| pub used: bool, | ||
| /// Was the symbol marked as `#[rustc_std_internal_symbol]`? | ||
| pub rustc_std_internal_symbol: bool, |
There was a problem hiding this comment.
The only difference between used and rustc_std_internal_symbol is whether LTO will export the symbol or not. used will result in the symbol being exported, while rustc_std_internal_symbol won't.
|
@rustbot ready The last two commits are new. |
|
☔ The latest upstream changes (presumably #141061) made this pull request unmergeable. Please resolve the merge conflicts. |
rustc_std_internal_symbol is meant to call functions from crates where there is no direct dependency on said crate. As they either have to be added to symbols.o or rustc has to introduce an implicit dependency on them to avoid linker errors. The latter is done for some things like the panic runtime, but adding these symbols to symbols.o allows removing those implicit dependencies.
This used to be necessary for a correct linker order, but ever since the introduction of symbols.o adding the symbols in question to symbols.o would work just as well. We do still add dependencies on the panic runtime to the local crate, but not for #![needs_panic_runtime] crates. This also removes the runtime-depends-on-needs-runtime test. inject_dependency_if used to emit this error, but with symbols.o it is no longer important that there is no dependency and in fact it may be nice to have panic_abort and panic_unwind directly depend on libstd in the future for calling std::process::abort().
You shouldn't ever need to explicitly depend on it. And we weren't checking that the panic runtime used the correct panic strategy either.
There is already panic-unwind to enable it.
bjorn3
force-pushed
the
panic_runtime_cleanup
branch
from
b46f424 to
0819538
Compare
See the individual commits for more info.