[DONT MERGE] externally implementable items #146348

jdonszelmann · 2025-09-08T21:04:17Z

Supersedes #140010
Tracking issue: #125418

Getting started:

#![feature(eii)]

#[eii(eii1)]
pub fn decl1(x: u64) 
// body optional (it's the default)
{
    println!("default {x}");
}

// in another crate, maybe
#[eii1]
pub fn decl2(x: u64) {
    println!("explicit {x}");
}

fn main() {
    decl1(4);
}

jdonszelmann · 2025-09-08T21:12:23Z

compiler/rustc_passes/src/reachable.rs

        // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs.
        || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_COMPILER)
        || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER)
+        // Right now, the only way to get "foreign item symbol aliases" is by being an EII-implementation.


TODO: do we need this:

option 1: we do and implement our own reachability analysis based on it separate from RUSTC_STD_INTERNAL_SYMBOL.

~~option 2: we don't cause we also use RUSTC_STD_INTERNAL_SYMBOL on all EIIs. In that case we should rename STD_INTERNAL_SYMBOL~~

option 3: same as option 2 but maybe we shouldn't use RUSTC_STD_INTERNAL_SYMBOL in the first place; we leave it as-is and create a new flag thta's like it but specifically for EIIs and named something else like RUSTC_USED_BY_NONDIRECT_DEP_CRATE or whatever nicer name we can think of that (as long as it doesn't use STD since it's not really specific to that anymore)

despite STD_INTERNAL_SYMBOL working, and we can test with it for a bit, this is not the way we should do it. i.e. not option 2.

jdonszelmann · 2025-09-08T21:14:02Z

tests/ui/eii/default/local_crate_explicit.rs

+
+#[eii(eii1)]
+pub fn decl1(x: u64) {
+    //~^ WARN function `decl1` is never used


It took me a little bit to decide what is the expected behavior here. We might want to remove this warning. Defaults are in some way expected to be unused. However, here we can prove it because the explicit impl is in the same crate hence the warning.

removing the warning actually makes the code a tiny bit trickier

jdonszelmann · 2025-09-09T20:10:50Z

@bors2 try @rust-timer queue

[DONT MERGE] externally implementable items

rust-bors · 2025-09-09T22:16:01Z

☀️ Try build successful (CI)
Build commit: d5a6633 (d5a66337e06d3b77a1ab8f6793055535195c3ae9, parent: 364da5d88d772fa40fb20353443595385443ac25)

rust-timer · 2025-09-09T23:40:37Z

Finished benchmarking commit (d5a6633): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.1%, 1.5%]	19
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	44
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.0%	[-0.1%, -0.0%]	4
All ❌✅ (primary)	0.3%	[0.1%, 1.5%]	19

Max RSS (memory usage)

Results (primary 1.7%, secondary 3.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.6%, 2.3%]	6
Regressions ❌ (secondary)	4.7%	[1.1%, 11.2%]	16
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.9%, -1.2%]	2
All ❌✅ (primary)	1.7%	[0.6%, 2.3%]	6

Cycles

Results (primary 3.1%, secondary -1.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	3.1%	[3.1%, 3.1%]	1
Regressions ❌ (secondary)	3.2%	[2.5%, 3.6%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.8%	[-4.1%, -2.0%]	9
All ❌✅ (primary)	3.1%	[3.1%, 3.1%]	1

Binary size

Results (primary 0.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.0%	[0.0%, 0.1%]	93
Regressions ❌ (secondary)	0.1%	[0.0%, 0.3%]	61
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.1%]	93

Bootstrap: 468.052s -> 471.14s (0.66%)
Artifact size: 387.52 MiB -> 390.25 MiB (0.71%)

jdonszelmann · 2025-09-10T02:44:26Z

Well, shit. I think I know some fixes but I hoped this wouldn't happen

compiler/rustc_hir_analysis/src/check/wfcheck.rs

tests/ui/eii/auxiliary/codegen1.rs

tests/ui/eii/cross_crate_wrong_ty.stderr

oli-obk · 2025-10-25T08:54:27Z

compiler/rustc_ast/src/ast.rs

+    /// This function is an implementation of an externally implementable item (EII).
+    /// This means, there was an EII declared somewhere and this function is the
+    /// implementation that should be run when the declaration is called.
+    pub eii_impls: ThinVec<EiiImpl>,


Why is it a vec and not an option?

one function could be the implementation of multiple EIIs. A panic handler could also be the integer overflow handler simultaneously

Hmm... Doesn't sound too compelling to support, but if the complexity is basically just in the collector/checker (where it's just a loop, so not a problem) and not in the macros or such, then it's ok.

Def needs tests tho. Also a test using the same eii attribute twice. Or one function defining two different eiis at the same time

compiler/rustc_ast/src/ast.rs

oli-obk · 2025-10-25T09:05:16Z

compiler/rustc_builtin_macros/src/eii.rs

+        span,
+        vis: ast::Visibility { span, kind: ast::VisibilityKind::Inherited, tokens: None },
+        kind: ast::ItemKind::ForeignMod(ast::ForeignMod {
+            extern_span: span,


Could be computed from the fn's extern span, everything but the generated Rust has one.

Various other uses of the fallback span may also have better spans to be used

compiler/rustc_builtin_macros/src/eii.rs

oli-obk · 2025-10-25T10:07:04Z

compiler/rustc_resolve/src/ident.rs

        ribs: &[Rib<'ra>],
        ignore_binding: Option<NameBinding<'ra>>,
    ) -> Option<LexicalScopeBinding<'ra>> {
-        assert!(ns == TypeNS || ns == ValueNS);


Are there tests for various situations with naming conflicts?

hm, unsure, In theory there shouldn't be any issue, EII attributes are simply part of the macro namespace and act exactly as other macros. The reason this assertion had to be removed is that we use the defid of that macro as an identifier for the EII itself - with is we look up its signature when type checking for example. And we used to effectively assert that all defids of macros were filtered out at this point.

compiler/rustc_metadata/src/eii.rs

+
+    // iterate over all items in the current crate
+    // FIXME(speed up)
+    for id in tcx.hir_crate_items(()).definitions() {


compiler/rustc_metadata/src/eii.rs

+
+    // iterate over all items in the current crate
+    // FIXME(speed up)
+    for id in tcx.hir_crate_items(()).definitions() {


jdonszelmann · 2025-10-27T10:55:56Z

@antoyo @GuillaumeGomez I already talked to Guillaume; CI is failing here due to a debug assertion about pointer casts. Guillaume was somewhat confused by this, thought it should already have been fixed. In the last commit (5e51a43), I disabled the relevant tests on the gcc backend, but I think it should just work. Mind taking a look?

antoyo · 2025-10-27T11:33:53Z

CI is failing here

This link points to a comment, not a CI. Could you please send the GitHub Actions link?

jdonszelmann · 2025-10-27T12:26:18Z

@antoyo https://github.com/rust-lang/rust/actions/runs/18317140124/job/52160440565

antoyo · 2025-10-27T12:28:57Z

@antoyo https://github.com/rust-lang/rust/actions/runs/18317140124/job/52160440565

I only see tidy checks failure in this, not the debug assertion.

jdonszelmann · 2025-10-27T12:31:18Z

I'm so sorry, I'm at the complete wrong comment. This is the one https://github.com/rust-lang/rust/actions/runs/18712544823/job/53364286179

jdonszelmann · 2025-10-27T12:33:31Z

oh! there the assertion is also not failing, what?

jdonszelmann · 2025-10-27T12:34:05Z

turns out the issue I was getting locally wasn't the same as in CI.... shit, I'm sorry for taking your time. Still don't get why not... but it's not the assertion at least...

antoyo · 2025-10-27T12:37:56Z

turns out the issue I was getting locally wasn't the same as in CI.... shit, I'm sorry for taking your time. Still don't get why not... but it's not the assertion at least...

No worries.
Could you try setting NO_DEBUG_ASSERTIONS=1 locally to see if this is what's missing from the doc. Perhaps setting rust.debug-assertions = false is not enough since I also had some issues and I ended up needing to set both. If that works for you with the env var, we could fix the doc.

Also, cg_gcc does not handle some attributes yet, so maybe this is the issue here.

…pe checking

EII collection queries