Skip to content

Commit

Permalink
Rollup merge of #95292 - BGR360:const-trait-specialize, r=lcnr
Browse files Browse the repository at this point in the history
Allow specialized const trait impls.

Fixes #95186.
Fixes #95187.

I've done my best to create a comprehensive test suite for the interaction between `min_specialization` and `const_trait_impls`. I wouldn't be surprised if there are interesting cases I haven't tested, please let me know.
  • Loading branch information
Manishearth authored Nov 11, 2022
2 parents 742d3f0 + 94f67e6 commit cd30ccf
Show file tree
Hide file tree
Showing 12 changed files with 382 additions and 25 deletions.
120 changes: 98 additions & 22 deletions compiler/rustc_hir_analysis/src/impl_wf_check/min_specialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use crate::constrained_generic_params as cgp;
use crate::errors::SubstsOnOverriddenImpl;

use rustc_data_structures::fx::FxHashSet;
use rustc_hir as hir;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::outlives::env::OutlivesEnvironment;
use rustc_infer::infer::TyCtxtInferExt;
Expand All @@ -80,6 +81,7 @@ use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt;
use rustc_trait_selection::traits::outlives_bounds::InferCtxtExt as _;
use rustc_trait_selection::traits::{self, translate_substs, wf, ObligationCtxt};
use tracing::instrument;

pub(super) fn check_min_specialization(tcx: TyCtxt<'_>, impl_def_id: LocalDefId) {
if let Some(node) = parent_specialization_node(tcx, impl_def_id) {
Expand All @@ -103,13 +105,11 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Opti
}

/// Check that `impl1` is a sound specialization
#[instrument(level = "debug", skip(tcx))]
fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node) {
if let Some((impl1_substs, impl2_substs)) = get_impl_substs(tcx, impl1_def_id, impl2_node) {
let impl2_def_id = impl2_node.def_id();
debug!(
"check_always_applicable(\nimpl1_def_id={:?},\nimpl2_def_id={:?},\nimpl2_substs={:?}\n)",
impl1_def_id, impl2_def_id, impl2_substs
);
debug!(?impl2_def_id, ?impl2_substs);

let parent_substs = if impl2_node.is_from_trait() {
impl2_substs.to_vec()
Expand All @@ -118,12 +118,33 @@ fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node
};

let span = tcx.def_span(impl1_def_id);
check_constness(tcx, impl1_def_id, impl2_node, span);
check_static_lifetimes(tcx, &parent_substs, span);
check_duplicate_params(tcx, impl1_substs, &parent_substs, span);
check_predicates(tcx, impl1_def_id, impl1_substs, impl2_node, impl2_substs, span);
}
}

/// Check that the specializing impl `impl1` is at least as const as the base
/// impl `impl2`
fn check_constness(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node, span: Span) {
if impl2_node.is_from_trait() {
// This isn't a specialization
return;
}

let impl1_constness = tcx.constness(impl1_def_id.to_def_id());
let impl2_constness = tcx.constness(impl2_node.def_id());

if let hir::Constness::Const = impl2_constness {
if let hir::Constness::NotConst = impl1_constness {
tcx.sess
.struct_span_err(span, "cannot specialize on const impl with non-const impl")
.emit();
}
}
}

/// Given a specializing impl `impl1`, and the base impl `impl2`, returns two
/// substitutions `(S1, S2)` that equate their trait references. The returned
/// types are expressed in terms of the generics of `impl1`.
Expand Down Expand Up @@ -278,15 +299,15 @@ fn check_static_lifetimes<'tcx>(

/// Check whether predicates on the specializing impl (`impl1`) are allowed.
///
/// Each predicate `P` must be:
/// Each predicate `P` must be one of:
///
/// * global (not reference any parameters)
/// * `T: Tr` predicate where `Tr` is an always-applicable trait
/// * on the base `impl impl2`
/// * Currently this check is done using syntactic equality, which is
/// conservative but generally sufficient.
/// * a well-formed predicate of a type argument of the trait being implemented,
/// * Global (not reference any parameters).
/// * A `T: Tr` predicate where `Tr` is an always-applicable trait.
/// * Present on the base impl `impl2`.
/// * This check is done using the `trait_predicates_eq` function below.
/// * A well-formed predicate of a type argument of the trait being implemented,
/// including the `Self`-type.
#[instrument(level = "debug", skip(tcx))]
fn check_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
impl1_def_id: LocalDefId,
Expand Down Expand Up @@ -322,10 +343,7 @@ fn check_predicates<'tcx>(
.map(|obligation| obligation.predicate)
.collect()
};
debug!(
"check_always_applicable(\nimpl1_predicates={:?},\nimpl2_predicates={:?}\n)",
impl1_predicates, impl2_predicates,
);
debug!(?impl1_predicates, ?impl2_predicates);

// Since impls of always applicable traits don't get to assume anything, we
// can also assume their supertraits apply.
Expand Down Expand Up @@ -373,25 +391,83 @@ fn check_predicates<'tcx>(
);

for (predicate, span) in impl1_predicates {
if !impl2_predicates.contains(&predicate) {
if !impl2_predicates.iter().any(|pred2| trait_predicates_eq(tcx, predicate, *pred2, span)) {
check_specialization_on(tcx, predicate, span)
}
}
}

/// Checks if some predicate on the specializing impl (`predicate1`) is the same
/// as some predicate on the base impl (`predicate2`).
///
/// This basically just checks syntactic equivalence, but is a little more
/// forgiving since we want to equate `T: Tr` with `T: ~const Tr` so this can work:
///
/// ```ignore (illustrative)
/// #[rustc_specialization_trait]
/// trait Specialize { }
///
/// impl<T: Bound> Tr for T { }
/// impl<T: ~const Bound + Specialize> const Tr for T { }
/// ```
///
/// However, we *don't* want to allow the reverse, i.e., when the bound on the
/// specializing impl is not as const as the bound on the base impl:
///
/// ```ignore (illustrative)
/// impl<T: ~const Bound> const Tr for T { }
/// impl<T: Bound + Specialize> const Tr for T { } // should be T: ~const Bound
/// ```
///
/// So we make that check in this function and try to raise a helpful error message.
fn trait_predicates_eq<'tcx>(
tcx: TyCtxt<'tcx>,
predicate1: ty::Predicate<'tcx>,
predicate2: ty::Predicate<'tcx>,
span: Span,
) -> bool {
let pred1_kind = predicate1.kind().skip_binder();
let pred2_kind = predicate2.kind().skip_binder();
let (trait_pred1, trait_pred2) = match (pred1_kind, pred2_kind) {
(ty::PredicateKind::Trait(pred1), ty::PredicateKind::Trait(pred2)) => (pred1, pred2),
// Just use plain syntactic equivalence if either of the predicates aren't
// trait predicates or have bound vars.
_ => return predicate1 == predicate2,
};

let predicates_equal_modulo_constness = {
let pred1_unconsted =
ty::TraitPredicate { constness: ty::BoundConstness::NotConst, ..trait_pred1 };
let pred2_unconsted =
ty::TraitPredicate { constness: ty::BoundConstness::NotConst, ..trait_pred2 };
pred1_unconsted == pred2_unconsted
};

if !predicates_equal_modulo_constness {
return false;
}

// Check that the predicate on the specializing impl is at least as const as
// the one on the base.
match (trait_pred2.constness, trait_pred1.constness) {
(ty::BoundConstness::ConstIfConst, ty::BoundConstness::NotConst) => {
tcx.sess.struct_span_err(span, "missing `~const` qualifier for specialization").emit();
}
_ => {}
}

true
}

#[instrument(level = "debug", skip(tcx))]
fn check_specialization_on<'tcx>(tcx: TyCtxt<'tcx>, predicate: ty::Predicate<'tcx>, span: Span) {
debug!("can_specialize_on(predicate = {:?})", predicate);
match predicate.kind().skip_binder() {
// Global predicates are either always true or always false, so we
// are fine to specialize on.
_ if predicate.is_global() => (),
// We allow specializing on explicitly marked traits with no associated
// items.
ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: _,
}) => {
ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _, polarity: _ }) => {
if !matches!(
trait_predicate_kind(tcx, predicate),
Some(TraitSpecializationKind::Marker)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Tests that trait bounds on specializing trait impls must be `~const` if the
// same bound is present on the default impl and is `~const` there.

#![feature(const_trait_impl)]
#![feature(rustc_attrs)]
#![feature(min_specialization)]

#[rustc_specialization_trait]
trait Specialize {}

#[const_trait]
trait Foo {}

#[const_trait]
trait Bar {}

// bgr360: I was only able to exercise the code path that raises the
// "missing ~const qualifier" error by making this base impl non-const, even
// though that doesn't really make sense to do. As seen below, if the base impl
// is made const, rustc fails earlier with an overlapping impl failure.
impl<T> Bar for T
where
T: ~const Foo,
{}

impl<T> Bar for T
where
T: Foo, //~ ERROR missing `~const` qualifier
T: Specialize,
{}

#[const_trait]
trait Baz {}

impl<T> const Baz for T
where
T: ~const Foo,
{}

impl<T> const Baz for T //~ ERROR conflicting implementations of trait `Baz`
where
T: Foo,
T: Specialize,
{}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
error: missing `~const` qualifier for specialization
--> $DIR/const-default-bound-non-const-specialized-bound.rs:28:8
|
LL | T: Foo,
| ^^^

error[E0119]: conflicting implementations of trait `Baz`
--> $DIR/const-default-bound-non-const-specialized-bound.rs:40:1
|
LL | impl<T> const Baz for T
| ----------------------- first implementation here
...
LL | impl<T> const Baz for T
| ^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0119`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Tests that a const default trait impl can be specialized by another const
// trait impl and that the specializing impl will be used during const-eval.

// run-pass

#![feature(const_trait_impl)]
#![feature(min_specialization)]

#[const_trait]
trait Value {
fn value() -> u32;
}

const fn get_value<T: ~const Value>() -> u32 {
T::value()
}

impl<T> const Value for T {
default fn value() -> u32 {
0
}
}

struct FortyTwo;

impl const Value for FortyTwo {
fn value() -> u32 {
42
}
}

const ZERO: u32 = get_value::<()>();

const FORTY_TWO: u32 = get_value::<FortyTwo>();

fn main() {
assert_eq!(ZERO, 0);
assert_eq!(FORTY_TWO, 42);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Tests that specializing trait impls must be at least as const as the default impl.

#![feature(const_trait_impl)]
#![feature(min_specialization)]

#[const_trait]
trait Value {
fn value() -> u32;
}

impl<T> const Value for T {
default fn value() -> u32 {
0
}
}

struct FortyTwo;

impl Value for FortyTwo { //~ ERROR cannot specialize on const impl with non-const impl
fn value() -> u32 {
println!("You can't do that (constly)");
42
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: cannot specialize on const impl with non-const impl
--> $DIR/const-default-impl-non-const-specialized-impl.rs:19:1
|
LL | impl Value for FortyTwo {
| ^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// check-pass

#![feature(const_trait_impl)]
#![feature(min_specialization)]

#[const_trait]
trait Foo {
fn foo();
}

impl const Foo for u32 {
default fn foo() {}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Tests that `~const` trait bounds can be used to specialize const trait impls.

// check-pass

#![feature(const_trait_impl)]
#![feature(rustc_attrs)]
#![feature(min_specialization)]

#[const_trait]
#[rustc_specialization_trait]
trait Specialize {}

#[const_trait]
trait Foo {}

impl<T> const Foo for T {}

impl<T> const Foo for T
where
T: ~const Specialize,
{}

#[const_trait]
trait Bar {}

impl<T> const Bar for T
where
T: ~const Foo,
{}

impl<T> const Bar for T
where
T: ~const Foo,
T: ~const Specialize,
{}

fn main() {}
Loading

0 comments on commit cd30ccf

Please sign in to comment.