Add #[rustc_edition_redirect]

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[Amanieu]

View all comments

This implements the compiler portion of the library API evolution project goal by adding the #[rustc_edition_redirect] attribute. This attribute allows an item path in the standard library to be redirected to a different item when used from a crate with an older edition.

This PR only implements the compiler portion and doesn't make any use of this in the standard library. However I do have a POC branch which replaces the edition-specific panic! dispatching with this.

Example

// stdpubstructOlder;pubstructOld;pubstructCurrent;#[rustc_edition_redirect = "2018"]pubuseOlderasCurrent;#[rustc_edition_redirect = "2021"]pubuseOldasCurrent;// 2024 edition crateuse std::Current;// resolves to Current// 2021 edition crateuse std::Current;// resolves to Old// 2018 edition crateuse std::Current;// resolves to Older

Semantics

• #[rustc_edition_redirect = "EDITION"] is only allowed on a single-item use.
• The annotated import is resolved and checked like an ordinary import, but does not introduce its alias while compiling the defining crate. Instead, it is attached as an edition-dependent alternative to the ordinary item or re-export with the same name and namespace.
• Every redirect requires a default item with the same name and visibility.
• Multiple redirects may be attached to the same default item, but duplicate edition boundaries are rejected.
• When an external name is looked up, the first redirect whose boundary is later than the edition of the lookup span is selected. If none applies, the default item is used.
• The edition comes from the span performing the lookup, so macro-generated identifiers retain the edition of their originating macro.
• Redirect selection applies to explicit paths, ordinary and glob imports, #[macro_use], and prelude lookup.
• Redirects are consumed by the first cross-crate lookup. If another crate re-exports the result, that re-export is fixed according to the edition of the re-exporting crate; redirect metadata is not propagated further.
• Since redirect targets are ordinary re-exports, normal visibility, stability, and import checks apply.

Implementation

1. Attribute parsing records only the edition boundary; the redirect target is the target of the annotated use.
2. Redirect imports participate in normal import resolution, but their aliases are not planted into the defining module.
3. After import resolution reaches its fixed point, redirects are grouped with their default declarations and checked for a missing default, duplicate boundaries, and visibility mismatches.
4. Resolved redirects are stored as part of ModChild in crate metadata.
5. External name-resolution paths use redirect-aware accessors which select a declaration using the edition of the lookup span.

Open questions

• This is currently gated under edition_redirect without a tracking issue. Does this need a separate tracking issue?
• Do we need a way to automatically propagate redirects through re-exports? Currently this must be done manually when re-exporting through core/alloc/std.
• Are ordinary re-export privacy and stability rules sufficient for legacy redirect targets that should not otherwise appear in the current-edition API?
• This redirect is currently not shown in rustdoc at all. Ideally we would want a note that links to what this resolves to in older editions.

r? petrochenkov

[rustbot]

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

[mejrs]

I can review the attribute portion of this.

• Multiple attributes are allowed with different before keys. The oldest one that applies is selected.

Given that it's an internal attribute it's not so important, but another option is a range syntax to force unambiguity (error on overlapping ranges):

#[rustc_edition_redirect(during = "..=2018", target(oldest_module))]#[rustc_edition_redirect(during = "2021..=2024", target(middle_module))]pubmod redirected_module {}

View changes since this review

[mejrs]

Suggested change

	if ast::attr::contains_name(&item.attrs, sym::rustc_edition_redirect)
	&& let Some(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =
	ifletSome(Attribute::Parsed(AttributeKind::RustcEditionRedirect(mut redirects))) =

No need for this check, the attribute parser already does it.

[Amanieu]

I tried removing this but it actually regressed performance: attribute parsing still does a lot of work even if the attribute is not found.

[mejrs]

This check should be in attribute parsing, not here. CombineParser::finalize_check can be overridden to do this. If you implement AttributeParser you can store state in your attribute parser instead, if that makes things easier.

[mejrs]

What is the behavior of before = "future"? Should it be supported or be an error? Either way it looks like there are no tests for it.

[Amanieu]

before = "future" should be supported just like any other edition.

[petrochenkov]

@bors try @rust-timer queue

[petrochenkov]

Given that this is a major expansion to name resolution, it's not surprising that I don't like it :)
My goal here will be making it less major while trying to still satisfy the library goals, perhaps at cost of some reduced ergonomics.

Some initial ideas:

• #[rustc_edition_redirect] is effectively a second reexport system parallel to the regular reexports, it even has to duplicate some reexport checks like "too private to reexport".
  I would prefer the "redirects" to reuse actual reexports, like

  #[rustc_edition_redirect = <edition_info>]useRedirectTargetasName;

  This would be resolved and checked exactly like a regular reexport, except it wouldn't plant Name into the current module to avoid conflicts.
  This will also allow to avoid resolving paths inside built-in attributes, something that was deliberately avoided so far.

• Second, #[rustc_edition_redirect = ...] attributes on a single item should be able to give a precise list of editions they target, in isolation.
  So we don't need to find the full set of items with redirects and the same name and check how their edition ranges overlap to get the edition list.
  Right now name resolution works the same in the local crate and in other crates, this feature as implemented breaks that invariant, but it would be good to keep it, or produce errors in cases where the resolution would works differently.
  #[rustc_edition_redirect = ...] use RedirectTarget as Name; could actually plant the Name into the current module if the rustc_edition_redirect matches the current local edition.

• Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?
  In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual. Another crate-level attribute gated by rustc_attrs would work instead (#[rustc_preserve_edition_redirects] or something).

I'll need to think about this more in the background for some time.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 1b93257 (1b9325785afb0e4413270e6bc497cf74b4bddc70)
Base parent: 922325b (922325bb13bfea5b41454318563f2a65e83c2336)

[rust-timer]

Finished benchmarking commit (1b93257): comparison URL.

Overall result: ❌✅ regressions and improvements - 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.4%	[0.2%, 0.7%]	49
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	50
Improvements ✅ (primary)	-0.4%	[-0.4%, -0.4%]	3
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	2
All ❌✅ (primary)	0.4%	[-0.4%, 0.7%]	52

Max RSS (memory usage)

Results (primary 0.8%, secondary 1.5%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.5%, 1.3%]	18
Regressions ❌ (secondary)	2.0%	[0.4%, 10.6%]	39
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.1%	[-4.5%, -0.6%]	4
All ❌✅ (primary)	0.8%	[0.5%, 1.3%]	18

Cycles

Results (primary 0.1%, 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)	1.0%	[0.5%, 2.4%]	5
Regressions ❌ (secondary)	1.7%	[0.4%, 6.1%]	23
Improvements ✅ (primary)	-1.4%	[-2.3%, -0.7%]	3
Improvements ✅ (secondary)	-4.7%	[-9.2%, -2.0%]	9
All ❌✅ (primary)	0.1%	[-2.3%, 2.4%]	8

Binary size

Results (primary 0.3%, 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.3%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	30
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.3%	[0.0%, 0.9%]	64

Bootstrap: 490.333s -> 491.013s (0.14%)
Artifact size: 392.58 MiB -> 390.64 MiB (-0.50%)

[Amanieu]

@petrochenkov Can you give more concrete examples of what you are proposing? You seem to be saying that we should just have multiple items with the same name in the same namespace with a different edition filter on them. I think that would be more complex and intrusive than the current solution.

#[rustc_edition_redirect = "2024"]useRedirectTarget2024asName;#[rustc_edition_redirect = "2021"]useRedirectTarget2021asName;#[rustc_edition_redirect = "2018"]useRedirectTarget2018asName;

The current system is much simpler: there is only one name per namespace and you can attach any number of edition redirects on that name:

#[rustc_edition_redirect(before = "2024", target(RedirectTarget2024)]
#[rustc_edition_redirect(before = "2021", target(RedirectTarget2021)]
#[rustc_edition_redirect(before = "2018", target(RedirectTarget2018)]structName;

That way all the redirection metadata for one name is available on the single ModChild for that name.

Third, is it possible to get rid of the "some crates propagate redirects, and some do not" feature, and always either propagate or not? Perhaps at cost of some mild inconveniences in the standard library?

In theory yes, we could manually add edition redirect attributes on every re-export of an item that has redirects. But that would just end up being completely equivalent to what the current implementation is doing, while being more error-prone since we may accidentally forget redirects in some places.

• In any case, right now the choice shouldn't be made by looking at whether feature(edition_redirect) is enabled or not, a missing feature is not supposed to change behavior, only to report a non-fatal error and then continue as usual.

There's quite a few places in the compiler where we change global behavior depending on whether a feature is enabled. For example stage_api forces all public items to have stability attributes. I feel that what I am doing is not too different, and in any case this feature is only intended for use in the standard library and tests.

[rust-timer]

Finished benchmarking commit (95ae99b): 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.4%	[0.2%, 1.4%]	11
Regressions ❌ (secondary)	0.4%	[0.1%, 0.8%]	38
Improvements ✅ (primary)	-0.3%	[-0.3%, -0.3%]	1
Improvements ✅ (secondary)	-0.2%	[-0.2%, -0.1%]	5
All ❌✅ (primary)	0.3%	[-0.3%, 1.4%]	12

Max RSS (memory usage)

Results (primary -0.0%, secondary -0.6%)

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

	mean	range	count
Regressions ❌ (primary)	0.6%	[0.4%, 1.0%]	6
Regressions ❌ (secondary)	2.5%	[0.5%, 5.1%]	10
Improvements ✅ (primary)	-3.6%	[-3.6%, -3.6%]	1
Improvements ✅ (secondary)	-4.0%	[-7.2%, -0.5%]	9
All ❌✅ (primary)	-0.0%	[-3.6%, 1.0%]	7

Cycles

Results (primary 0.2%, secondary 0.9%)

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

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.4%, 1.3%]	8
Regressions ❌ (secondary)	1.2%	[0.4%, 5.0%]	16
Improvements ✅ (primary)	-1.3%	[-2.3%, -0.6%]	3
Improvements ✅ (secondary)	-1.8%	[-3.1%, -0.6%]	2
All ❌✅ (primary)	0.2%	[-2.3%, 1.3%]	11

Binary size

Results (primary 0.2%, 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.2%	[0.0%, 0.9%]	64
Regressions ❌ (secondary)	0.1%	[0.0%, 0.5%]	26
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.2%	[0.0%, 0.9%]	64

Bootstrap: 491.018s -> 490.892s (-0.03%)
Artifact size: 390.29 MiB -> 390.30 MiB (0.00%)

[petrochenkov]

Here's some minimized version that I came up with today - petrochenkov@2659b90.
(It needs a further refactoring, but the "name resolution logic" part is minimized.)
In this version I actually have some confidence.

Initially I wanted to limit all the edition-based dispatch to one place in resolve_ident_in_extern_module_non_globs_unadjusted, to make it truly cross-crate only.
For this the "propagation of redirects through imports" had to be removed, because it brings the edition-based dispatch into the current crate in a complicated way which is error-prone and in which I had no confidence.

But then I realized that one place in resolve_ident_in_extern_module_non_globs_unadjusted is still error-prone, because any code that uses self.resolutions(...) or self.resolution(...).non_glob_decl can easily get the non-redirected version of the extern declaration using the same key!

So I hid the NameResolution structure into a module and made the non_glob_decl field private, and added methods that ensure that we either get the redirected version of the binding (fn non_glob_decl_redir), or panic if the redirect set is non-empty (fn non_glob_decl).
With this all the tests pass except these cases that test the "redirect propagation" specifically.
(I think it's better if we instead do stuff like this explicitly in the standard library for now.)

// tests\ui\edition-redirect\basic.rslet _:ExpectedScopedRedirected = edition_redirect::ScopedRedirected;let _: edition_redirect::same_redirects::Item = ExpectedRedirectedUse;// tests\ui\edition-redirect\reexport.rslet _: reexport_preserving::Item = reexport_source::current();let _: reexport_preserving::Child = reexport_source::current_child();

All the other things work correctly because the correct redirects are fetched in resolve_ident_in_extern_module_non_globs_unadjusted, resolve_glob_import, and for_each_child_mut (for macro_use), and then automatically propagate through all the following imports, etc.

[rustbot]

Some changes occurred in compiler/rustc_attr_ir

cc @jdonszelmann, @JonathanBrouwer

[Amanieu]

@petrochenkov I applied your changes on top of mine and then had an LLM audit every single code path that ends up calling non_glob_decl. This exposed some issues that are addressed in the latest commit:

• We need redir and non-redir versions of for_each_child, depending on the caller's context. Notably, add_module_candidates and lookup_import_candidates_from_module.
• Diagnostic lookups need to use the redir version because they may be looking up an item in the standard library that has redirects. The non-redir version panics if it is called on a decl with redirects.
• The per-module trait cache needs to be keyed by edition because the set of available traits in a module may depend on the edition used to do a lookup into that module. This is relevant when resolving a trait through the prelude, which (unlike glob imports) performs the resolution using the edition of the place it is used rather than the edition of the glob import itself.

Tests were added for all of these issues. However I'm not 100% confident about these, so another round of review is probably necessary.

[Amanieu]

cc @rust-lang/edition for awareness.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.