Add force option for --extern flag

[mhammerly]

When --extern force:foo=libfoo.so is passed to rustc and foo is not actually used in the crate, inject an extern crate foo; statement into the AST force it to be resolved anyway in CrateLoader::postprocess(). This allows you to, for instance, inject a #[panic_handler] implementation into a #![no_std] crate without modifying its source so that it can be built as a dylib. It may also be useful for #![panic_runtime] or #[global_allocator]/#![default_lib_allocator] implementations.

My work previously involved integrating Rust into an existing C/C++ codebase which was built with Buck and shipped on, among other platforms, Android. When targeting Android, Buck builds all "native" code with shared linkage* so it can be loaded from Java/Kotlin. My project was not itself #![no_std], but many of our dependencies were, and they would fail to build with shared linkage due to a lack of a panic handler. With this change, that project can add the new force option to the std dependency it already explicitly provides to every crate to solve this problem.

*This is an oversimplification - Buck has a couple features for aggregating dependencies into larger shared libraries, but none that I think sustainably solve this problem.

The AST injection happens after macro expansion around where we similarly inject a test harness and proc-macro harness. The resolver's list of actually-used extern flags is populated during macro expansion, and if any of our --extern arguments have the force option and weren't already used, we inject an extern crate statement for them. The injection logic was added in rustc_builtin_macros as that's where similar injections for tests, proc-macros, and std/core already live.

(New contributor - grateful for feedback and guidance!)

[rustbot]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @jackh726 (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

[clubby789]

I believe adding a new option like this requires a Major Change Proposal

[mhammerly]

A similar change (#96067) didn't go through that process afaict, but if that's what's appropriate I'm happy to do it! cc @ehuss who seems to have implemented the first --extern options in #67074 - what do you think?

[ehuss]

I'm not on the compiler team, but I believe the MCP is the process nowadays.

[est31]

This will probably remain a buck-only feature, as the one added by #96067 was too. As in, for cargo the problem should already solved because even if you specify some crate to be compiled as cdylib, the crates it depends on aren't. Also, cargo doesn't specify any explicit extern param for depending on std, using the sysroot mechanism instead.

I guess this feature is the crate-level equivalent of the #[used] attribute, so there is precedent both for the feature as well as options/flags on --extern params.

[mhammerly]

@rustbot blocked

i'll create an MCP and then bump for review later

EDIT: MCP rust-lang/compiler-team#605

[petrochenkov]

Injecting extern crate statements is a pretty large hammer that may have unintended consequences.
I'm not actually sure that the current implementation works as written, extern crate items are processed during expansion and if they are inserted after it they are left unprocessed (unless some error recovery logic processes them?).

I suggest directly calling CrateLoader::resolve_crate on the crates from the --extern force list instead.
Probably somewhere inside CrateLoader::postprocess.

[mhammerly]

@petrochenkov I think the injected crates are processed fully; following the lead of test_harness.rs and proc_macro_harness.rs, extern_crate_statement::inject() calls cx.monotonic_expander().fully_expand_fragment(fragment).

That said, I went with the AST approach because it seemed like a "complete" injection and I worried merely calling resolve_crate() (which is all that is done for the profiler runtime too) could cause some non-obvious bug down the line. It sounds like you don't expect any issues, so if someone seconds the MCP I can make sure that approach works and update the PR before bumping for review!

As an aside: with std/core/compiler_builtins, test, and proc-macro injections happening around AST expansion and profiler_runtime, panic_runtime, and allocator shim injection happening in CrateLoader::postprocess(), do you think it's worth a separate change to establish a unified pattern?

[petrochenkov]

std/core/compiler_builtins

User code can refer to these (even to compiler_builtins on 2015 edition, if I'm not mistaken), so these are injected before doing any crate expansion.

compiler_builtins can probably be moved to postprocess with minimal breakage though, it's only there for linking.

test and proc-macro

These need to be in AST/HIR (or at least it's the simplest way to achieve the desired effect).
Also they just generate some regular code, not link to other crates.

profiler_runtime, panic_runtime, and allocator shim injection happening

These are added only for linking side effect, so they can be added directly to CStore without any AST.

[mhammerly]

thanks for those details! that all makes sense.

[mhammerly]

the MCP was seconded a week ago so I updated the PR to take @petrochenkov's suggestion: skip the extern crate statement and just call CrateLoader::resolve_crate() instead. the test cases I added before still work

@rustbot review

[mhammerly]

@rustbot review

thanks for the feedback!

[mhammerly]

alright, x86_64-msvc-2 is passing and the output indicates everything passed:

test result: ok. 14729 passed; 0 failed; 174 ignored; 0 measured; 0 filtered out; finished in 865.30s

the CI configuration + my copy of #111141 has been backed out.
@rustbot review

unlike the previous failed run, these logs don't print a line for each test so i can't explicitly confirm my tests ran, but this run has the same number of "ignored" tests as the failing one and everything else passed so we're probably good?

i apologize for the noisy iterations, but thanks to y'all for the support!

[petrochenkov]

@bors r+

[bors]

📌 Commit 9a0ee5fc73128c11b6b4657d86ab76c9f5f5ffc0 has been approved by petrochenkov

It is now in the queue for this repository.

[bors]

⌛ Testing commit 9a0ee5fc73128c11b6b4657d86ab76c9f5f5ffc0 with merge ed03c68bf1780fb1a8050b6a57d3f177895e781a...

[bors]

💔 Test failed - checks-actions

[mhammerly]

alright, this time i looked into the libc crate like i said i would. most targets (not windows!) pass -nodefaultlibs to the linker, and when building a crate against std it pulls in the libc crate which has #[link()] attributes for necessary system libs including msvcrt and libc. because our panic_handler dylib is #![no_std], we need to depend on the libc crate from the sysroot explicitly. i took the approach from other tests.

as windows doesn't pass -nodefaultlibs, i don't actually know why it was failing. and i'm also not sure why linux builds weren't failing. nevertheless, all three platforms are passing now:

• x86_64-msvc-2
• x86_64-apple-2
• x86_64-gnu-llvm-14

this latest run was rebased past the x.ps1 fix and the CI changes have been backed out.
@rustbot review

process question: is there a tracking issue template i should use for this? i can't edit #98405 to link to a new issue, but i would be happy to file one and update the docs in this PR or a follow-up PR to follow suit with the other --extern options

[petrochenkov]

@bors r+

[bors]

📌 Commit 812f2d7 has been approved by petrochenkov

It is now in the queue for this repository.

[petrochenkov]

@mhammerly

i can't edit #98405 to link to a new issue, but i would be happy to file one and update the docs in this PR or a follow-up PR to follow suit with the other --extern options

You can create a new tracking issue using those linked from #98405 as examples, and I'll then update #98405 to link to it.
Updating docs would also be nice.

[bors]

⌛ Testing commit 812f2d7 with merge 333b920...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 333b920 to master...

[rust-timer]

Finished benchmarking commit (333b920): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.7%	[-2.8%, -0.8%]	4
Improvements ✅ (secondary)	-5.2%	[-7.5%, -0.2%]	7
All ❌✅ (primary)	-1.7%	[-2.8%, -0.8%]	4

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	1.1%	[1.1%, 1.1%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.3%	[-2.3%, -2.3%]	1
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.5%	[3.5%, 3.5%]	1
Improvements ✅ (primary)	-1.6%	[-1.6%, -1.6%]	1
Improvements ✅ (secondary)	-4.8%	[-6.1%, -3.9%]	6
All ❌✅ (primary)	-1.6%	[-1.6%, -1.6%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 653.613s -> 656.005s (0.37%)

[mhammerly]

@jsgf @pcwalton i've been in touch with @capickett about this separately, but you may also be interested in this PR. it lets you build vendored #![no_std] crates as dylibs by force-injecting a dependency on a crate that includes a panic handler (such as std). it'll be in 1.71.0 i think