[NaitikVerma6776]

Fixes #158227.

#[export_stable] currently performs its exportability checks only for sdylib crates. This change moves the crate-type check after the exportability validation so the same checks are also performed for non-sdylib crate types.

Non-sdylib crates still return an empty exportable-items list, so their existing export behavior is unchanged.

Updated the relevant UI tests to cover the validation on non-sdylib crate types.

[rustbot]

Thanks for the pull request, and welcome! The Rust Project has assigned @nnethercote (or someone else) to review your changes, you should hear from them (or someone else) within the next two weeks.

Please see the contribution instructions and our LLM policy for more information.

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 77 candidates
• Random selection from 17 candidates

[JonathanBrouwer]

@bors try @rust-timer queue

[bjorn3]

Should we not deny #[export_stable] entirely for non-sdylib crate types?

[rust-bors]

☀️ Try build successful (CI)
Build commit: bd16461 (bd164614d5ed566dcd7139bab46ae040d7ff6cc0)
Base parent: b57eb9a (b57eb9a5fd94f30697f51fc88fbe898e67c7aae6)

[rust-timer]

Finished benchmarking commit (bd16461): comparison URL.

Overall result: ❌ regressions - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

@bors rollup=never rustc-perf
@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.1%	[0.1%, 0.1%]	6
Regressions ❌ (secondary)	0.5%	[0.5%, 0.5%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.1%, 0.1%]	6

Max RSS (memory usage)

Results (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)	-	-	0
Regressions ❌ (secondary)	1.5%	[1.5%, 1.5%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.7%	[-2.9%, -2.5%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results (primary -3.0%, secondary -4.5%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	2
Improvements ✅ (secondary)	-4.5%	[-11.2%, -1.8%]	12
All ❌✅ (primary)	-3.0%	[-3.0%, -3.0%]	2

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.693s -> 489.31s (-0.08%)
Artifact size: 409.33 MiB -> 409.30 MiB (-0.01%)

[NaitikVerma6776]

hii @JonathanBrouwer I’ve pushed the follow-up changes to address the CI failures and the changes only update the affected .stderr expectations based on the CI output; the implementation itself is unchanged .....CI should rerun now and I’d also would like to know your thoughts on the question about whether #[export_stable] should be denied entirely for non-sdylib crate types as mentioned by @bjorn3

[mejrs]

Should we not deny #[export_stable] entirely for non-sdylib crate types?

How does this work if you want to build multiple crate type outputs for your crate? Would you need to create build scripts to emit cfgs and use #[cfg_attr(building_sdylib_cfg, export_stable)] everywhere?

That sounds extremely annoying. I think it would be more practical if it's a no-op on non-sdylib crates but otherwise has the same errors and restrictions.

In any case, such a decision should be part of the RFC.

[bjorn3]

We could allow it if any of the crate types passed to rustc is sdylib. But even so I'm not sure if allowing sdylib together with other crate types is ok. If I recall correctly sdylib was messing with symbol mangling, which would break other crate types.

[mejrs]

Another option is a lint, such as for

#![warn(linker_messages)]

warning: unused attribute
 --> src/lib.rs:1:1
  |
1 | #![warn(linker_messages)]
  | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: remove this attribute
  |
  = note: the `linker_messages` and `linker_info` lints can only be controlled at the root of a crate that needs to be linked
  = note: `#[warn(unused_attributes)]` (part of `#[warn(unused)]`) on by default

[mejrs]

whether #[export_stable] should be denied entirely for non-sdylib crate types as mentioned by @bjorn3

that question is orthogonal to this pr, if we decide to make this an error we should still display all the target errors as well.

[NaitikVerma6776]

sdylib is currently allows alongside other crate types, but I couldn't find tests covering combinations like sdylib,rlib......it also affects the export symbol mangling path........Might be worth a separate follow-up investigation.......though I agree this seems orthogonal to this PR.

[bjorn3]

that question is orthogonal to this pr, if we decide to make this an error we should still display all the target errors as well.

The #[proc_macro] family of attributes is similar to #[export_stable]. For those it seems we check the target item kind in the attribute parser, but don't validate anything else for non-proc-macro crate types. We don't emit any diagnostic for using it on a private item unless inside a proc-macro crate for example.

[JonathanBrouwer]

Should we not deny #[export_stable] entirely for non-sdylib crate types?

If I understand the RFC correctly,
a library can be used both as a normal rust library, and a sdylib. Even if the library is not currently being built as an sdylib, the attributes should be legal.

Normally, when using a crate as a dependency, any #[export] attributes of that crate have no effect and the dependency is statically linked into the resulting binary.

Literally interpreting "have no effect" would mean completely ignoring them, but I don't think that's the intention of the original text. I think the intention is a no-op but with the same errors and restrictions.

In my opinion, not validating attributes just because they're not being used is a bad idea. Library authors might make the mistake to not build their library as an sdylib in their CI, and then miss that they're using the attributes wrong. I don't see why we should accept code that is incorrect.

But even so I'm not sure if allowing sdylib together with other crate types is ok. If I recall correctly sdylib was messing with symbol mangling, which would break other crate types.

This is a separate issue than the one this PR is solving, if you can make a reproducer for it I'd say open an issue

[JonathanBrouwer]

Could you short circuit the function to return an empty slice if the feature gate for export_stable is disabled? That should solve the perf regression

View changes since the review

[mejrs]

This would just be a temporary fix, right? I'm assuming we'll want to stabilize this at some point. Is it possible to run this query based on whether #[export_stable] is present anywhere in the crate?

[JonathanBrouwer]

That's true but I think that keeping track of whether export_stable exists is a lot more code to maintain for a feature that is probably not getting stabilized anytime soon. When this feature is closer to stabilization we can reconsider doing this

[JonathanBrouwer]

The perf effect is relatively small as well so it's probably just never worth the code

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[rust-bors]

☔ The latest upstream changes (presumably #165151) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.