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.

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

Min specialization improvements #111252

Merged
merged 3 commits into from
May 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions compiler/rustc_data_structures/src/owned_slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,11 @@ impl Borrow<[u8]> for OwnedSlice {
}

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Box<dyn Send + Sync>)`, which is `Send`
#[cfg(parallel_compiler)]
unsafe impl Send for OwnedSlice {}

// Safety: `OwnedSlice` is conceptually `(&'self.1 [u8], Box<dyn Send + Sync>)`, which is `Sync`
#[cfg(parallel_compiler)]
unsafe impl Sync for OwnedSlice {}

#[cfg(test)]
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,9 @@ hir_analysis_specialization_trait = implementing `rustc_specialization_trait` tr
hir_analysis_closure_implicit_hrtb = implicit types in closure signatures are forbidden when `for<...>` is present
.label = `for<...>` is here

hir_analysis_empty_specialization = specialization impl does not specialize any associated items
.note = impl is a specialization of this impl

hir_analysis_const_specialize = cannot specialize on const impl with non-const impl

hir_analysis_static_specialize = cannot specialize on `'static` lifetime
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -803,6 +803,15 @@ pub(crate) struct ClosureImplicitHrtb {
pub for_sp: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_empty_specialization)]
pub(crate) struct EmptySpecialization {
#[primary_span]
pub span: Span,
#[note]
pub base_impl_span: Span,
}

#[derive(Diagnostic)]
#[diag(hir_analysis_const_specialize)]
pub(crate) struct ConstSpecialize {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ use rustc_middle::ty::{self, TyCtxt, TypeVisitableExt};
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 rustc_trait_selection::traits::{self, translate_substs_with_cause, wf, ObligationCtxt};

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 @@ -100,12 +100,19 @@ fn parent_specialization_node(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId) -> Opti
// Implementing a normal trait isn't a specialization.
return None;
}
if trait_def.is_marker {
// Overlapping marker implementations are not really specializations.
return None;
}
Some(impl2_node)
}

/// 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) {
let span = tcx.def_span(impl1_def_id);
check_has_items(tcx, impl1_def_id, impl2_node, span);

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!(?impl2_def_id, ?impl2_substs);
Expand All @@ -116,14 +123,20 @@ fn check_always_applicable(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node
unconstrained_parent_impl_substs(tcx, impl2_def_id, impl2_substs)
};

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);
}
}

fn check_has_items(tcx: TyCtxt<'_>, impl1_def_id: LocalDefId, impl2_node: Node, span: Span) {
if let Node::Impl(impl2_id) = impl2_node && tcx.associated_item_def_ids(impl1_def_id).is_empty() {
let base_impl_span = tcx.def_span(impl2_id);
tcx.sess.emit_err(errors::EmptySpecialization { span, base_impl_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) {
Expand Down Expand Up @@ -167,8 +180,21 @@ fn get_impl_substs(
ocx.assumed_wf_types(param_env, tcx.def_span(impl1_def_id), impl1_def_id);

let impl1_substs = InternalSubsts::identity_for_item(tcx, impl1_def_id);
let impl2_substs =
translate_substs(infcx, param_env, impl1_def_id.to_def_id(), impl1_substs, impl2_node);
let impl1_span = tcx.def_span(impl1_def_id);
let impl2_substs = translate_substs_with_cause(
infcx,
param_env,
impl1_def_id.to_def_id(),
impl1_substs,
impl2_node,
|_, span| {
traits::ObligationCause::new(
impl1_span,
impl1_def_id,
traits::ObligationCauseCode::BindingObligation(impl2_node.def_id(), span),
)
},
);

let errors = ocx.select_all_or_error();
if !errors.is_empty() {
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2728,8 +2728,6 @@ pub struct UserTypeProjection {
pub projs: Vec<ProjectionKind>,
}

impl Copy for ProjectionKind {}

impl UserTypeProjection {
pub(crate) fn index(mut self) -> Self {
self.projs.push(ProjectionElem::Index(()));
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_trait_selection/src/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ fn negative_impl(tcx: TyCtxt<'_>, impl1_def_id: DefId, impl2_def_id: DefId) -> b
let selcx = &mut SelectionContext::new(&infcx);
let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id);
let (subject2, obligations) =
impl_subject_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs);
impl_subject_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs, |_, _| {
ObligationCause::dummy()
});

!equate(&infcx, impl_env, subject1, subject2, obligations, impl1_def_id)
}
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ pub use self::select::{EvaluationCache, SelectionCache, SelectionContext};
pub use self::select::{EvaluationResult, IntercrateAmbiguityCause, OverflowError};
pub use self::specialize::specialization_graph::FutureCompatOverlapError;
pub use self::specialize::specialization_graph::FutureCompatOverlapErrorKind;
pub use self::specialize::{specialization_graph, translate_substs, OverlapError};
pub use self::specialize::{
specialization_graph, translate_substs, translate_substs_with_cause, OverlapError,
};
pub use self::structural_match::{
search_for_adt_const_param_violation, search_for_structural_match_violation,
};
Expand Down
69 changes: 51 additions & 18 deletions compiler/rustc_trait_selection/src/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,30 @@ pub fn translate_substs<'tcx>(
source_impl: DefId,
source_substs: SubstsRef<'tcx>,
target_node: specialization_graph::Node,
) -> SubstsRef<'tcx> {
translate_substs_with_cause(
infcx,
param_env,
source_impl,
source_substs,
target_node,
|_, _| ObligationCause::dummy(),
)
}

/// Like [translate_substs], but obligations from the parent implementation
/// are registered with the provided `ObligationCause`.
///
/// This is for reporting *region* errors from those bounds. Type errors should
/// not happen because the specialization graph already checks for those, and
/// will result in an ICE.
pub fn translate_substs_with_cause<'tcx>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
source_impl: DefId,
source_substs: SubstsRef<'tcx>,
target_node: specialization_graph::Node,
cause: impl Fn(usize, Span) -> ObligationCause<'tcx>,
) -> SubstsRef<'tcx> {
debug!(
"translate_substs({:?}, {:?}, {:?}, {:?})",
Expand All @@ -99,14 +123,13 @@ pub fn translate_substs<'tcx>(
return source_substs;
}

fulfill_implication(infcx, param_env, source_trait_ref, target_impl).unwrap_or_else(
|()| {
fulfill_implication(infcx, param_env, source_trait_ref, source_impl, target_impl, cause)
.unwrap_or_else(|()| {
bug!(
"When translating substitutions from {source_impl:?} to {target_impl:?}, \
the expected specialization failed to hold"
)
},
)
})
}
specialization_graph::Node::Trait(..) => source_trait_ref.substs,
};
Expand Down Expand Up @@ -153,20 +176,12 @@ pub(super) fn specializes(tcx: TyCtxt<'_>, (impl1_def_id, impl2_def_id): (DefId,

// Create an infcx, taking the predicates of impl1 as assumptions:
let infcx = tcx.infer_ctxt().build();
let impl1_trait_ref =
match traits::fully_normalize(&infcx, ObligationCause::dummy(), penv, impl1_trait_ref) {
Ok(impl1_trait_ref) => impl1_trait_ref,
Err(_errors) => {
tcx.sess.delay_span_bug(
tcx.def_span(impl1_def_id),
format!("failed to fully normalize {impl1_trait_ref}"),
);
impl1_trait_ref
}
};

// Attempt to prove that impl2 applies, given all of the above.
fulfill_implication(&infcx, penv, impl1_trait_ref, impl2_def_id).is_ok()
fulfill_implication(&infcx, penv, impl1_trait_ref, impl1_def_id, impl2_def_id, |_, _| {
ObligationCause::dummy()
})
.is_ok()
}

/// Attempt to fulfill all obligations of `target_impl` after unification with
Expand All @@ -178,23 +193,41 @@ fn fulfill_implication<'tcx>(
infcx: &InferCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
source_trait_ref: ty::TraitRef<'tcx>,
source_impl: DefId,
target_impl: DefId,
error_cause: impl Fn(usize, Span) -> ObligationCause<'tcx>,
) -> Result<SubstsRef<'tcx>, ()> {
debug!(
"fulfill_implication({:?}, trait_ref={:?} |- {:?} applies)",
param_env, source_trait_ref, target_impl
);

let source_trait_ref = match traits::fully_normalize(
&infcx,
ObligationCause::dummy(),
param_env,
source_trait_ref,
) {
Ok(source_trait_ref) => source_trait_ref,
Err(_errors) => {
infcx.tcx.sess.delay_span_bug(
infcx.tcx.def_span(source_impl),
format!("failed to fully normalize {source_trait_ref}"),
);
source_trait_ref
}
};

let source_trait = ImplSubject::Trait(source_trait_ref);

let selcx = &mut SelectionContext::new(&infcx);
let target_substs = infcx.fresh_substs_for_item(DUMMY_SP, target_impl);
let (target_trait, obligations) =
util::impl_subject_and_oblig(selcx, param_env, target_impl, target_substs);
util::impl_subject_and_oblig(selcx, param_env, target_impl, target_substs, error_cause);

// do the impls unify? If not, no specialization.
let Ok(InferOk { obligations: more_obligations, .. }) =
infcx.at(&ObligationCause::dummy(), param_env, ).eq(DefineOpaqueTypes::No,source_trait, target_trait)
infcx.at(&ObligationCause::dummy(), param_env).eq(DefineOpaqueTypes::No, source_trait, target_trait)
else {
debug!(
"fulfill_implication: {:?} does not unify with {:?}",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ pub fn impl_subject_and_oblig<'a, 'tcx>(
param_env: ty::ParamEnv<'tcx>,
impl_def_id: DefId,
impl_substs: SubstsRef<'tcx>,
cause: impl Fn(usize, Span) -> ObligationCause<'tcx>,
) -> (ImplSubject<'tcx>, impl Iterator<Item = PredicateObligation<'tcx>>) {
let subject = selcx.tcx().impl_subject(impl_def_id);
let subject = subject.subst(selcx.tcx(), impl_substs);
Expand All @@ -208,8 +209,7 @@ pub fn impl_subject_and_oblig<'a, 'tcx>(
let predicates = predicates.instantiate(selcx.tcx(), impl_substs);
let InferOk { value: predicates, obligations: normalization_obligations2 } =
selcx.infcx.at(&ObligationCause::dummy(), param_env).normalize(predicates);
let impl_obligations =
super::predicates_for_generics(|_, _| ObligationCause::dummy(), param_env, predicates);
let impl_obligations = super::predicates_for_generics(cause, param_env, predicates);

let impl_obligations = impl_obligations
.chain(normalization_obligations1.into_iter())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ trait Specialize {}
trait Foo {}

#[const_trait]
trait Bar {}
trait Bar {
fn 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
Expand All @@ -21,26 +23,36 @@ trait Bar {}
impl<T> Bar for T
where
T: ~const Foo,
{}
{
default fn bar() {}
}

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

#[const_trait]
trait Baz {}
trait Baz {
fn baz();
}

impl<T> const Baz for T
where
T: ~const Foo,
{}
{
default fn baz() {}
}

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

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

error[E0119]: conflicting implementations of trait `Baz`
--> $DIR/const-default-bound-non-const-specialized-bound.rs:40:1
--> $DIR/const-default-bound-non-const-specialized-bound.rs:50:1
|
LL | impl<T> const Baz for T
| ----------------------- first implementation here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,39 @@
trait Specialize {}

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

impl<T> const Foo for T {}
impl<T> const Foo for T {
default fn foo() {}
}

impl<T> const Foo for T
where
T: ~const Specialize,
{}
{
fn foo() {}
}

#[const_trait]
trait Bar {}
trait Bar {
fn bar() {}
}

impl<T> const Bar for T
where
T: ~const Foo,
{}
{
default fn bar() {}
}

impl<T> const Bar for T
where
T: ~const Foo,
T: ~const Specialize,
{}
{
fn bar() {}
}

fn main() {}
Loading