Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stabilize feature(trait_upcasting) #134367

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+76 −389
Open

Stabilize feature(trait_upcasting) #134367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c032527
stabilize `feature(trait_upcasting)`
WaffleLapkin Dec 16, 2024
24c0bf4
chage `deref_into_dyn_supertrait` lint wording
WaffleLapkin Dec 16, 2024
fdd5ba8
remove `feature(trait_upcasting)` from tests and bless them
WaffleLapkin Dec 16, 2024
a5e3c91
remove unstable book entry for `feature(trait_upcasting)`
WaffleLapkin Dec 16, 2024
ccd1db0
remove use of `feature(trait_upcasting)` from core tests
WaffleLapkin Dec 16, 2024
8563da8
remove use of `feature(trait_upcasting)` from miri
WaffleLapkin Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions compiler/rustc_feature/src/accepted.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,9 @@ declare_features! (
/// Allows `#[track_caller]` to be used which provides
/// accurate caller location reporting during panic (RFC 2091).
(accepted, track_caller, "1.46.0", Some(47809)),
/// Allows dyn upcasting trait objects via supertraits.
/// Dyn upcasting is casting, e.g., `dyn Foo -> dyn Bar` where `Foo: Bar`.
(accepted, trait_upcasting, "CURRENT_RUSTC_VERSION", Some(65991)),
/// Allows #[repr(transparent)] on univariant enums (RFC 2645).
(accepted, transparent_enums, "1.42.0", Some(60405)),
/// Allows indexing tuples.
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_feature/src/unstable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,9 +623,6 @@ declare_features! (
(unstable, thread_local, "1.0.0", Some(29594)),
/// Allows defining `trait X = A + B;` alias items.
(unstable, trait_alias, "1.24.0", Some(41517)),
/// Allows dyn upcasting trait objects via supertraits.
/// Dyn upcasting is casting, e.g., `dyn Foo -> dyn Bar` where `Foo: Bar`.
(unstable, trait_upcasting, "1.56.0", Some(65991)),
/// Allows for transmuting between arrays with sizes that contain generic consts.
(unstable, transmute_generic_consts, "1.70.0", Some(109929)),
/// Allows #[repr(transparent)] on unions (RFC 2645).
Expand Down
23 changes: 0 additions & 23 deletions compiler/rustc_hir_typeck/src/coercion.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
)];

let mut has_unsized_tuple_coercion = false;
let mut has_trait_upcasting_coercion = None;

// Keep resolving `CoerceUnsized` and `Unsize` predicates to avoid
// emitting a coercion in cases like `Foo<$1>` -> `Foo<$2>`, where
Expand Down Expand Up @@ -682,13 +681,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
// these here and emit a feature error if coercion doesn't fail
// due to another reason.
match impl_source {
traits::ImplSource::Builtin(
BuiltinImplSource::TraitUpcasting { .. },
_,
) => {
has_trait_upcasting_coercion =
Some((trait_pred.self_ty(), trait_pred.trait_ref.args.type_at(1)));
}
traits::ImplSource::Builtin(BuiltinImplSource::TupleUnsizing, _) => {
has_unsized_tuple_coercion = true;
}
Expand All @@ -699,21 +691,6 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
}
}

if let Some((sub, sup)) = has_trait_upcasting_coercion
&& !self.tcx().features().trait_upcasting()
{
// Renders better when we erase regions, since they're not really the point here.
let (sub, sup) = self.tcx.erase_regions((sub, sup));
let mut err = feature_err(
&self.tcx.sess,
sym::trait_upcasting,
self.cause.span,
format!("cannot cast `{sub}` to `{sup}`, trait upcasting coercion is experimental"),
);
err.note(format!("required when coercing `{source}` into `{target}`"));
err.emit();
}

if has_unsized_tuple_coercion && !self.tcx.features().unsized_tuple_coercion() {
feature_err(
&self.tcx.sess,
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_lint/src/deref_into_dyn_supertrait.rs
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not actually sure what to do with this lint. "Future incompatible" doesn't make sense anymore, since this PR is actually adding the breaking change... should I make it into a normal lint? Should I make it deny-by-default?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be time to do a crater run for switching it to deny-by-default.

The conservative case is to keep it warn and switch it later - I think that's okay.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Particularly, I could very much imagine people having these impls around today precisely because trait upcasting isn't stable. And so switching it later might be much easier.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Future incompatible" doesn't make sense anymore, since this PR is actually adding the breaking change

So we can't keep this a lint and proceed with compilation when it triggers?

Copy link
Member

@RalfJung RalfJung Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait this is a FutureReleaseSemanticsChange, I've never seen one of those... I see now. Yeah probably the lint should just be removed, but Cc @rust-lang/lang to be sure. This is about #89460.

Copy link
Member Author

@WaffleLapkin WaffleLapkin Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think disallowing the impl is silly, it can be useful, for example in combination with genetic code. I'm fine with making this a normal lint or removing the lint altogether.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I nominate for T-lang discussion again, since there doesn't seem to be a consensus of what to do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that's good

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW we currently do not warn for Deref impls that are shadowed by other kinds of coercions, for example:

array->slice unsizing example 
use std::ops::Deref;

struct Wrap<T: ?Sized>(T);

impl Deref for Wrap<[u8; 0]> {
    type Target = Wrap<[u8]>;

    fn deref(&self) -> &Self::Target {
        println!("deref!");
        self
    }
}

fn main() {
    let _: &Wrap<[u8]> = &Wrap([]); // doesn't print "deref!"
    let _: &Wrap<[u8]> = Deref::deref(&Wrap([])); // prints "deref!"
}
subtyping example 
use std::ops::Deref;

struct Wrap<T: ?Sized>(T);

impl Deref for Wrap<fn(&())> {
    type Target = Wrap<fn(&'static ())>;

    fn deref(&self) -> &Self::Target {
        println!("deref!");
        self
    }
}

fn main() {
    let _: &Wrap<fn(&'static ())> = &Wrap((|_| ()) as fn(&())); // doesn't print "deref!"
    let _: &Wrap<fn(&'static ())> = Deref::deref(&Wrap((|_| ()) as fn(&()))); // prints "deref!"
}

So IMO if we keep this lint, it should apply to all Deref impls that are shadowed by any kind of coercion and not be arbitrarily restricted to the specific Deref impls that will no longer be called after this PR merges.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we go that round I'd prefer to remove the lint & leave implementing the full shadowed_deref lint as a followup.

Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ declare_lint! {
/// The `deref_into_dyn_supertrait` lint is output whenever there is a use of the
/// `Deref` implementation with a `dyn SuperTrait` type as `Output`.
///
/// These implementations will become shadowed when the `trait_upcasting` feature is stabilized.
/// The `deref` functions will no longer be called implicitly, so there might be behavior change.
/// These implementations are shadowed by the `trait_upcasting` feature (stabilized since
/// CURRENT_RUSTC_VERSION). The `deref` functions is no longer called implicitly, which might
/// be behavior change compared to previous rustc versions.
///
/// ### Example
///
Expand Down Expand Up @@ -43,11 +44,11 @@ declare_lint! {
///
/// ### Explanation
///
/// The dyn upcasting coercion feature adds new coercion rules, taking priority
/// over certain other coercion rules, which will cause some behavior change.
/// The dyn upcasting coercion feature added a new coercion rules, taking priority
/// over certain other coercion rules, which caused some behavior change.
pub DEREF_INTO_DYN_SUPERTRAIT,
Warn,
"`Deref` implementation usage with a supertrait trait object for output might be shadowed in the future",
"`Deref` implementation usage with a supertrait trait object for output is shadowed by trait upcasting",
@future_incompatible = FutureIncompatibleInfo {
reason: FutureIncompatibilityReason::FutureReleaseSemanticsChange,
reference: "issue #89460 <https://github.com/rust-lang/rust/issues/89460>",
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

// tidy-alphabetical-start
#![allow(internal_features)]
#![cfg_attr(bootstrap, feature(trait_upcasting))]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![doc(rust_logo)]
#![feature(array_windows)]
Expand All @@ -32,7 +33,6 @@
#![feature(let_chains)]
#![feature(rustc_attrs)]
#![feature(rustdoc_internals)]
#![feature(trait_upcasting)]
#![warn(unreachable_pub)]
// tidy-alphabetical-end

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#![allow(rustc::diagnostic_outside_of_impl)]
#![allow(rustc::potential_query_instability)]
#![allow(rustc::untranslatable_diagnostic)]
#![cfg_attr(bootstrap, feature(trait_upcasting))]
#![doc(html_root_url = "https://doc.rust-lang.org/nightly/nightly-rustc/")]
#![doc(rust_logo)]
#![feature(allocator_api)]
Expand Down Expand Up @@ -56,7 +57,6 @@
#![feature(ptr_alignment_type)]
#![feature(rustc_attrs)]
#![feature(rustdoc_internals)]
#![feature(trait_upcasting)]
#![feature(trusted_len)]
#![feature(try_blocks)]
#![feature(try_trait_v2)]
Expand Down
63 changes: 1 addition & 62 deletions compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ use hir::LangItem;
use hir::def_id::DefId;
use rustc_data_structures::fx::{FxHashSet, FxIndexSet};
use rustc_hir as hir;
use rustc_infer::traits::{
Obligation, ObligationCause, PolyTraitObligation, PredicateObligations, SelectionError,
};
use rustc_infer::traits::{Obligation, PolyTraitObligation, SelectionError};
use rustc_middle::ty::fast_reject::DeepRejectCtxt;
use rustc_middle::ty::{self, ToPolyTraitRef, Ty, TypeVisitableExt, TypingMode};
use rustc_middle::{bug, span_bug};
Expand All @@ -23,8 +21,6 @@ use tracing::{debug, instrument, trace};

use super::SelectionCandidate::*;
use super::{BuiltinImplConditions, SelectionCandidateSet, SelectionContext, TraitObligationStack};
use crate::traits;
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::util;

impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
Expand Down Expand Up @@ -898,54 +894,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
})
}

/// Temporary migration for #89190
fn need_migrate_deref_output_trait_object(
&mut self,
ty: Ty<'tcx>,
param_env: ty::ParamEnv<'tcx>,
cause: &ObligationCause<'tcx>,
) -> Option<ty::PolyExistentialTraitRef<'tcx>> {
// Don't drop any candidates in intercrate mode, as it's incomplete.
// (Not that it matters, since `Unsize` is not a stable trait.)
//
// FIXME(@lcnr): This should probably only trigger during analysis,
// disabling candidates during codegen is also questionable.
if let TypingMode::Coherence = self.infcx.typing_mode() {
return None;
}

let tcx = self.tcx();
if tcx.features().trait_upcasting() {
return None;
}

// <ty as Deref>
let trait_ref = ty::TraitRef::new(tcx, tcx.lang_items().deref_trait()?, [ty]);

let obligation =
traits::Obligation::new(tcx, cause.clone(), param_env, ty::Binder::dummy(trait_ref));
if !self.infcx.predicate_may_hold(&obligation) {
return None;
}

self.infcx.probe(|_| {
let ty = traits::normalize_projection_ty(
self,
param_env,
ty::AliasTy::new_from_args(tcx, tcx.lang_items().deref_target()?, trait_ref.args),
cause.clone(),
0,
// We're *intentionally* throwing these away,
// since we don't actually use them.
&mut PredicateObligations::new(),
)
.as_type()
.unwrap();

if let ty::Dynamic(data, ..) = ty.kind() { data.principal() } else { None }
})
}

/// Searches for unsizing that might apply to `obligation`.
fn assemble_candidates_for_unsizing(
&mut self,
Expand Down Expand Up @@ -1014,15 +962,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let principal_a = a_data.principal().unwrap();
let target_trait_did = principal_def_id_b.unwrap();
let source_trait_ref = principal_a.with_self_ty(self.tcx(), source);
if let Some(deref_trait_ref) = self.need_migrate_deref_output_trait_object(
source,
obligation.param_env,
&obligation.cause,
) {
if deref_trait_ref.def_id() == target_trait_did {
return;
}
}

for (idx, upcast_trait_ref) in
util::supertraits(self.tcx(), source_trait_ref).enumerate()
Expand Down
2 changes: 1 addition & 1 deletion library/core/tests/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// tidy-alphabetical-start
#![cfg_attr(bootstrap, feature(trait_upcasting))]
#![cfg_attr(target_has_atomic = "128", feature(integer_atomics))]
#![cfg_attr(test, feature(cfg_match))]
#![feature(alloc_layout_extra)]
Expand Down Expand Up @@ -80,7 +81,6 @@
#![feature(strict_provenance_atomic_ptr)]
#![feature(strict_provenance_lints)]
#![feature(test)]
#![feature(trait_upcasting)]
#![feature(trusted_len)]
#![feature(trusted_random_access)]
#![feature(try_blocks)]
Expand Down
26 changes: 0 additions & 26 deletions src/doc/unstable-book/src/language-features/trait-upcasting.md
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion src/tools/miri/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![cfg_attr(bootstrap, feature(trait_upcasting))]
#![feature(rustc_private)]
#![feature(cell_update)]
#![feature(float_gamma)]
Expand All @@ -9,7 +10,6 @@
#![feature(yeet_expr)]
#![feature(nonzero_ops)]
#![feature(let_chains)]
#![feature(trait_upcasting)]
#![feature(strict_overflow_ops)]
#![feature(pointer_is_aligned_to)]
#![feature(unqualified_local_imports)]
Expand Down
3 changes: 0 additions & 3 deletions src/tools/miri/tests/fail/dyn-upcast-trait-mismatch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
// Validation stops this too early.
//@compile-flags: -Zmiri-disable-validation

#![feature(trait_upcasting)]
#![allow(incomplete_features)]

trait Foo: PartialEq<i32> + std::fmt::Debug + Send + Sync {
#[allow(dead_code)]
fn a(&self) -> i32 {
Expand Down
3 changes: 1 addition & 2 deletions src/tools/miri/tests/pass/box-custom-alloc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//@revisions: stack tree
//@[tree]compile-flags: -Zmiri-tree-borrows
#![allow(incomplete_features)] // for trait upcasting
#![feature(allocator_api, trait_upcasting)]
#![feature(allocator_api)]

use std::alloc::{AllocError, Allocator, Layout};
use std::cell::Cell;
Expand Down
3 changes: 0 additions & 3 deletions src/tools/miri/tests/pass/dyn-upcast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
#![feature(trait_upcasting)]
#![allow(incomplete_features)]

use std::fmt;

fn main() {
Expand Down
1 change: 0 additions & 1 deletion tests/codegen/vtable-upcast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//@ compile-flags: -C no-prepopulate-passes -Copt-level=0

#![crate_type = "lib"]
#![feature(trait_upcasting)]

pub trait Base {
fn base(&self);
Expand Down
2 changes: 1 addition & 1 deletion tests/crashes/131886.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ known-bug: #131886
//@ compile-flags: -Zvalidate-mir --crate-type=lib
#![feature(trait_upcasting, type_alias_impl_trait)]
#![feature(type_alias_impl_trait)]

type Tait = impl Sized;

Expand Down
1 change: 0 additions & 1 deletion tests/ui/codegen/issue-99551.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ build-pass
#![feature(trait_upcasting)]

pub trait A {}
pub trait B {}
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/dyn-star/no-unsize-coerce-dyn-trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#![feature(dyn_star, trait_upcasting)]
//~^ WARN the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
#![expect(incomplete_features)]
#![feature(dyn_star)]

trait A: B {}
trait B {}
Expand Down
11 changes: 1 addition & 10 deletions tests/ui/dyn-star/no-unsize-coerce-dyn-trait.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,3 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/no-unsize-coerce-dyn-trait.rs:1:12
|
LL | #![feature(dyn_star, trait_upcasting)]
| ^^^^^^^^
|
= note: see issue #102425 <https://github.com/rust-lang/rust/issues/102425> for more information
= note: `#[warn(incomplete_features)]` on by default

error[E0308]: mismatched types
--> $DIR/no-unsize-coerce-dyn-trait.rs:11:26
|
Expand All @@ -18,6 +9,6 @@ LL | let y: Box<dyn* B> = x;
= note: expected struct `Box<dyn* B>`
found struct `Box<dyn* A>`

error: aborting due to 1 previous error; 1 warning emitted
error: aborting due to 1 previous error

For more information about this error, try `rustc --explain E0308`.
2 changes: 1 addition & 1 deletion tests/ui/dyn-star/upcast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//@ known-bug: #104800

#![feature(dyn_star, trait_upcasting)]
#![feature(dyn_star)]

trait Foo: Bar {
fn hello(&self);
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/dyn-star/upcast.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
warning: the feature `dyn_star` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/upcast.rs:3:12
|
LL | #![feature(dyn_star, trait_upcasting)]
LL | #![feature(dyn_star)]
| ^^^^^^^^
|
= note: see issue #102425 <https://github.com/rust-lang/rust/issues/102425> for more information
Expand Down
13 changes: 0 additions & 13 deletions tests/ui/feature-gates/feature-gate-trait_upcasting.rs
View file
Open in desktop

This file was deleted.

14 changes: 0 additions & 14 deletions tests/ui/feature-gates/feature-gate-trait_upcasting.stderr
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion tests/ui/impl-trait/unsized_coercion5.old.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error[E0277]: the size for values of type `impl Trait + ?Sized` cannot be known at compilation time
--> $DIR/unsized_coercion5.rs:17:32
--> $DIR/unsized_coercion5.rs:15:32
|
LL | let y: Box<dyn Send> = x as Box<dyn Trait + Send>;
| ^ doesn't have a size known at compile-time
Expand Down
Loading
Loading