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!