From 614760f612fa72ba9d6955c00624c592b884d28f Mon Sep 17 00:00:00 2001 From: lcnr Date: Thu, 21 Sep 2023 08:56:23 +0200 Subject: [PATCH] review --- .../src/traits/coherence.rs | 153 +++++++++--------- 1 file changed, 75 insertions(+), 78 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index b4c0306ec60..59f712721fb 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -214,76 +214,69 @@ fn overlap<'tcx>( let mut obligations = equate_impl_headers(selcx.infcx, &impl1_header, &impl2_header)?; debug!("overlap: unification check succeeded"); - if !overlap_mode.use_implicit_negative() { - let impl_header = selcx.infcx.resolve_vars_if_possible(impl1_header); - return Some(OverlapResult { - impl_header, - intercrate_ambiguity_causes: Default::default(), - involves_placeholder: false, - }); - }; - obligations.extend( [&impl1_header.predicates, &impl2_header.predicates].into_iter().flatten().map( |&predicate| Obligation::new(infcx.tcx, ObligationCause::dummy(), param_env, predicate), ), ); - for mode in [TreatInductiveCycleAs::Ambig, TreatInductiveCycleAs::Recur] { - if let Some(failing_obligation) = selcx.with_treat_inductive_cycle_as(mode, |selcx| { - impl_intersection_has_impossible_obligation(selcx, &obligations) - }) { - if matches!(mode, TreatInductiveCycleAs::Recur) { - let first_local_impl = impl1_header - .impl_def_id - .as_local() - .or(impl2_header.impl_def_id.as_local()) - .expect("expected one of the impls to be local"); - infcx.tcx.struct_span_lint_hir( - COINDUCTIVE_OVERLAP_IN_COHERENCE, - infcx.tcx.local_def_id_to_hir_id(first_local_impl), - infcx.tcx.def_span(first_local_impl), - format!( - "implementations {} will conflict in the future", - match impl1_header.trait_ref { - Some(trait_ref) => { - let trait_ref = infcx.resolve_vars_if_possible(trait_ref); - format!( - "of `{}` for `{}`", - trait_ref.print_only_trait_path(), - trait_ref.self_ty() - ) - } - None => format!( - "for `{}`", - infcx.resolve_vars_if_possible(impl1_header.self_ty) - ), + if overlap_mode.use_implicit_negative() { + for mode in [TreatInductiveCycleAs::Ambig, TreatInductiveCycleAs::Recur] { + if let Some(failing_obligation) = selcx.with_treat_inductive_cycle_as(mode, |selcx| { + impl_intersection_has_impossible_obligation(selcx, &obligations) + }) { + if matches!(mode, TreatInductiveCycleAs::Recur) { + let first_local_impl = impl1_header + .impl_def_id + .as_local() + .or(impl2_header.impl_def_id.as_local()) + .expect("expected one of the impls to be local"); + infcx.tcx.struct_span_lint_hir( + COINDUCTIVE_OVERLAP_IN_COHERENCE, + infcx.tcx.local_def_id_to_hir_id(first_local_impl), + infcx.tcx.def_span(first_local_impl), + format!( + "implementations {} will conflict in the future", + match impl1_header.trait_ref { + Some(trait_ref) => { + let trait_ref = infcx.resolve_vars_if_possible(trait_ref); + format!( + "of `{}` for `{}`", + trait_ref.print_only_trait_path(), + trait_ref.self_ty() + ) + } + None => format!( + "for `{}`", + infcx.resolve_vars_if_possible(impl1_header.self_ty) + ), + }, + ), + |lint| { + lint.note( + "impls that are not considered to overlap may be considered to \ + overlap in the future", + ) + .span_label( + infcx.tcx.def_span(impl1_header.impl_def_id), + "the first impl is here", + ) + .span_label( + infcx.tcx.def_span(impl2_header.impl_def_id), + "the second impl is here", + ); + lint.note(format!( + "`{}` may be considered to hold in future releases, \ + causing the impls to overlap", + infcx.resolve_vars_if_possible(failing_obligation.predicate) + )); + lint }, - ), - |lint| { - lint.note( - "impls that are not considered to overlap may be considered to \ - overlap in the future", - ) - .span_label( - infcx.tcx.def_span(impl1_header.impl_def_id), - "the first impl is here", - ) - .span_label( - infcx.tcx.def_span(impl2_header.impl_def_id), - "the second impl is here", - ); - lint.note(format!( - "`{}` may be considered to hold in future releases, \ - causing the impls to overlap", - infcx.resolve_vars_if_possible(failing_obligation.predicate) - )); - lint - }, - ); - } + ); + } - return None; + return None; + } } } @@ -294,7 +287,9 @@ fn overlap<'tcx>( return None; } - let intercrate_ambiguity_causes = if infcx.next_trait_solver() { + let intercrate_ambiguity_causes = if !overlap_mode.use_implicit_negative() { + Default::default() + } else if infcx.next_trait_solver() { compute_intercrate_ambiguity_causes(&infcx, &obligations) } else { selcx.take_intercrate_ambiguity_causes() @@ -932,6 +927,8 @@ fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow tr.trait_ref, Some(ty::PredicateKind::Clause(ty::ClauseKind::Projection(proj))) => { @@ -942,21 +939,6 @@ fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow {} - // We only add intercrate ambiguity causes if the goal would - // otherwise result in an error. - // - // FIXME: this isn't quite right. Changing a goal from YES with - // inference contraints to AMBIGUOUS can also cause a goal to not - // fail. - Ok(Certainty::Yes) => { - ambiguity_cause = None; - break; - } - Err(NoSolution) => continue, - } - // FIXME: boiiii, using string comparisions here sure is scuffed. if let inspect::ProbeKind::MiscCandidate { name: "coherence unknowable", result: _ } = cand.kind() @@ -1011,6 +993,21 @@ fn visit_goal(&mut self, goal: &InspectGoal<'_, 'tcx>) -> ControlFlow { + ambiguity_cause = None; + break; + } + Err(NoSolution) => continue, + } } }