Skip to content

Commit

Permalink
Rollup merge of #111252 - matthewjasper:min-spec-improvements, r=comp…
Browse files Browse the repository at this point in the history
…iler-errors

Min specialization improvements

- Don't allow specialization impls with no items, such implementations are probably not correct and only occur as mistakes in the compiler and standard library
- Fix a missing normalization call
- Adds spans for lifetime errors from overly general specializations

Closes #79457
Closes #109815
  • Loading branch information
Dylan-DPC authored May 9, 2023
2 parents 8c51701 + f46eabb commit f748bb1
Show file tree
Hide file tree
Showing 20 changed files with 346 additions and 48 deletions.
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 @@ -279,6 +279,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 @@ -814,6 +814,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 @@ -55,7 +55,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

0 comments on commit f748bb1

Please sign in to comment.