Skip to content

Extract overlap_check_considering_specialization from insert #108577

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

Closed
wants to merge 2 commits into from
Closed

Extract overlap_check_considering_specialization from insert #108577

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40e82d9
Rename Inserted to OverlapResult
spastorino Feb 28, 2023
a73d57b
Extract overlap_check_considering_specialization from insert
spastorino Feb 28, 2023
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
233 changes: 125 additions & 108 deletions compiler/rustc_trait_selection/src/traits/specialize/specialization_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@ pub struct FutureCompatOverlapError<'tcx> {
pub kind: FutureCompatOverlapErrorKind,
}

/// The result of attempting to insert an impl into a group of children.
enum Inserted<'tcx> {
/// The impl was inserted as a new child in this group of children.
BecameNewSibling(Option<FutureCompatOverlapError<'tcx>>),
/// The result of overlap_check, we would attempt to insert an impl into a group of children
/// afterwards.
enum OverlapResult<'tcx> {
/// The impl is going to be inserted as a new child in this group of children.
NoOverlap(Option<FutureCompatOverlapError<'tcx>>),

/// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc.
ReplaceChildren(Vec<DefId>),
SpecializeAll(Vec<DefId>),

/// The impl is a specialization of an existing child.
ShouldRecurseOn(DefId),
SpecializeOne(DefId),
}

trait ChildrenExt<'tcx> {
Expand All @@ -42,7 +43,7 @@ trait ChildrenExt<'tcx> {
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted<'tcx>, OverlapError<'tcx>>;
) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>>;
}

impl<'tcx> ChildrenExt<'tcx> for Children {
Expand Down Expand Up @@ -86,128 +87,144 @@ impl<'tcx> ChildrenExt<'tcx> for Children {
impl_def_id: DefId,
simplified_self: Option<SimplifiedType>,
overlap_mode: OverlapMode,
) -> Result<Inserted<'tcx>, OverlapError<'tcx>> {
let mut last_lint = None;
let mut replace_children = Vec::new();

) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>> {
debug!("insert(impl_def_id={:?}, simplified_self={:?})", impl_def_id, simplified_self,);

let possible_siblings = match simplified_self {
Some(st) => PotentialSiblings::Filtered(filtered_children(self, st)),
None => PotentialSiblings::Unfiltered(iter_children(self)),
};

for possible_sibling in possible_siblings {
debug!(
"insert: impl_def_id={:?}, simplified_self={:?}, possible_sibling={:?}",
impl_def_id, simplified_self, possible_sibling,
);
let result = overlap_check_considering_specialization(
tcx,
impl_def_id,
possible_siblings,
overlap_mode,
);

let create_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>| {
let trait_ref = overlap.impl_header.trait_ref.unwrap();
let self_ty = trait_ref.self_ty();

OverlapError {
with_impl: possible_sibling,
trait_ref,
// Only report the `Self` type if it has at least
// some outer concrete shell; otherwise, it's
// not adding much information.
self_ty: self_ty.has_concrete_skeleton().then_some(self_ty),
intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
involves_placeholder: overlap.involves_placeholder,
}
};

let report_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>,
last_lint: &mut _| {
// Found overlap, but no specialization; error out or report future-compat warning.

// Do we *still* get overlap if we disable the future-incompatible modes?
let should_err = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::default(),
overlap_mode,
)
.is_some();

let error = create_overlap_error(overlap);

if should_err {
Err(error)
} else {
*last_lint = Some(FutureCompatOverlapError {
error,
kind: FutureCompatOverlapErrorKind::LeakCheck,
});

Ok((false, false))
}
};
if let Ok(OverlapResult::NoOverlap(_)) = result {
// No overlap with any potential siblings, so add as a new sibling.
debug!("placing as new sibling");
self.insert_blindly(tcx, impl_def_id);
}

let last_lint_mut = &mut last_lint;
let (le, ge) = traits::overlapping_impls(
result
}
}

fn overlap_check_considering_specialization<'tcx>(
tcx: TyCtxt<'tcx>,
impl_def_id: DefId,
possible_siblings: PotentialSiblings<impl Iterator<Item = DefId>, impl Iterator<Item = DefId>>,
overlap_mode: OverlapMode,
) -> Result<OverlapResult<'tcx>, OverlapError<'tcx>> {
let mut last_lint = None;
let mut replace_children = Vec::new();

for possible_sibling in possible_siblings {
debug!("insert: impl_def_id={:?}, possible_sibling={:?}", impl_def_id, possible_sibling);

let create_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>| {
let trait_ref = overlap.impl_header.trait_ref.unwrap();
let self_ty = trait_ref.self_ty();

OverlapError {
with_impl: possible_sibling,
trait_ref,
// Only report the `Self` type if it has at least
// some outer concrete shell; otherwise, it's
// not adding much information.
self_ty: self_ty.has_concrete_skeleton().then_some(self_ty),
intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
involves_placeholder: overlap.involves_placeholder,
}
};

let report_overlap_error = |overlap: traits::coherence::OverlapResult<'tcx>,
last_lint: &mut _| {
// Found overlap, but no specialization; error out or report future-compat warning.

// Do we *still* get overlap if we disable the future-incompatible modes?
let should_err = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::Yes,
traits::SkipLeakCheck::default(),
overlap_mode,
)
.map_or(Ok((false, false)), |overlap| {
if let Some(overlap_kind) =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
{
match overlap_kind {
ty::ImplOverlapKind::Permitted { marker: _ } => {}
ty::ImplOverlapKind::Issue33140 => {
*last_lint_mut = Some(FutureCompatOverlapError {
error: create_overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue33140,
});
}
}
.is_some();

let error = create_overlap_error(overlap);

return Ok((false, false));
if should_err {
Err(error)
} else {
*last_lint = Some(FutureCompatOverlapError {
error,
kind: FutureCompatOverlapErrorKind::LeakCheck,
});

Ok((false, false))
}
};

let last_lint_mut = &mut last_lint;
let (le, ge) = traits::overlapping_impls(
tcx,
possible_sibling,
impl_def_id,
traits::SkipLeakCheck::Yes,
overlap_mode,
)
.map_or(Ok((false, false)), |overlap| {
if let Some(overlap_kind) =
tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling)
{
match overlap_kind {
ty::ImplOverlapKind::Permitted { marker: _ } => {}
ty::ImplOverlapKind::Issue33140 => {
*last_lint_mut = Some(FutureCompatOverlapError {
error: create_overlap_error(overlap),
kind: FutureCompatOverlapErrorKind::Issue33140,
});
}
}

let le = tcx.specializes((impl_def_id, possible_sibling));
let ge = tcx.specializes((possible_sibling, impl_def_id));
return Ok((false, false));
}

if le == ge { report_overlap_error(overlap, last_lint_mut) } else { Ok((le, ge)) }
})?;
let le = tcx.specializes((impl_def_id, possible_sibling));
let ge = tcx.specializes((possible_sibling, impl_def_id));

if le && !ge {
debug!(
"descending as child of TraitRef {:?}",
tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
);
if le == ge { report_overlap_error(overlap, last_lint_mut) } else { Ok((le, ge)) }
})?;

// The impl specializes `possible_sibling`.
return Ok(Inserted::ShouldRecurseOn(possible_sibling));
} else if ge && !le {
debug!(
"placing as parent of TraitRef {:?}",
tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
);
if le && !ge {
debug!(
"descending as child of TraitRef {:?}",
tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
);

replace_children.push(possible_sibling);
} else {
// Either there's no overlap, or the overlap was already reported by
// `overlap_error`.
}
}
// The impl specializes `possible_sibling`.
return Ok(OverlapResult::SpecializeOne(possible_sibling));
} else if ge && !le {
debug!(
"placing as parent of TraitRef {:?}",
tcx.impl_trait_ref(possible_sibling).unwrap().subst_identity()
);

if !replace_children.is_empty() {
return Ok(Inserted::ReplaceChildren(replace_children));
replace_children.push(possible_sibling);
} else {
// Either there's no overlap, or the overlap was already reported by
// `overlap_error`.
}
}

// No overlap with any potential siblings, so add as a new sibling.
debug!("placing as new sibling");
self.insert_blindly(tcx, impl_def_id);
Ok(Inserted::BecameNewSibling(last_lint))
if !replace_children.is_empty() {
return Ok(OverlapResult::SpecializeAll(replace_children));
}

Ok(OverlapResult::NoOverlap(last_lint))
}

fn iter_children(children: &mut Children) -> impl Iterator<Item = DefId> + '_ {
Expand Down Expand Up @@ -306,7 +323,7 @@ impl<'tcx> GraphExt<'tcx> for Graph {

// Descend the specialization tree, where `parent` is the current parent node.
loop {
use self::Inserted::*;
use self::OverlapResult::*;

let insert_result = self.children.entry(parent).or_default().insert(
tcx,
Expand All @@ -316,11 +333,11 @@ impl<'tcx> GraphExt<'tcx> for Graph {
)?;

match insert_result {
BecameNewSibling(opt_lint) => {
NoOverlap(opt_lint) => {
last_lint = opt_lint;
break;
}
ReplaceChildren(grand_children_to_be) => {
SpecializeAll(grand_children_to_be) => {
// We currently have
//
// P
Expand Down Expand Up @@ -359,7 +376,7 @@ impl<'tcx> GraphExt<'tcx> for Graph {
}
break;
}
ShouldRecurseOn(new_parent) => {
SpecializeOne(new_parent) => {
parent = new_parent;
}
}
Expand Down