From 89fdb62331df00cf3f0fbd064b159d49d4d0fb48 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 17 Mar 2022 22:51:45 -0300 Subject: [PATCH 1/9] Fix inherent impls on negative coherence --- .../src/traits/coherence.rs | 106 +++++++++++------- .../coherence/coherence-negative-inherent.rs | 22 ++++ 2 files changed, 87 insertions(+), 41 deletions(-) create mode 100644 src/test/ui/coherence/coherence-negative-inherent.rs diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 94a4001bbb9..83f54d6dc3d 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -300,55 +300,79 @@ fn negative_impl<'cx, 'tcx>( debug!("negative_impl(impl1_def_id={:?}, impl2_def_id={:?})", impl1_def_id, impl2_def_id); let tcx = selcx.infcx().tcx; - // create a parameter environment corresponding to a (placeholder) instantiation of impl1 - let impl1_env = tcx.param_env(impl1_def_id); - let impl1_trait_ref = tcx.impl_trait_ref(impl1_def_id).unwrap(); - // Create an infcx, taking the predicates of impl1 as assumptions: tcx.infer_ctxt().enter(|infcx| { - // Normalize the trait reference. The WF rules ought to ensure - // that this always succeeds. - let impl1_trait_ref = match traits::fully_normalize( - &infcx, - FulfillmentContext::new(), - ObligationCause::dummy(), - impl1_env, - impl1_trait_ref, - ) { - Ok(impl1_trait_ref) => impl1_trait_ref, - Err(err) => { - bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); - } - }; + // create a parameter environment corresponding to a (placeholder) instantiation of impl1 + let impl1_env = tcx.param_env(impl1_def_id); - // Attempt to prove that impl2 applies, given all of the above. - let selcx = &mut SelectionContext::new(&infcx); - let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); - let (impl2_trait_ref, obligations) = - impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); + if let Some(impl1_trait_ref) = tcx.impl_trait_ref(impl1_def_id) { + // Normalize the trait reference. The WF rules ought to ensure + // that this always succeeds. + let impl1_trait_ref = match traits::fully_normalize( + &infcx, + FulfillmentContext::new(), + ObligationCause::dummy(), + impl1_env, + impl1_trait_ref, + ) { + Ok(impl1_trait_ref) => impl1_trait_ref, + Err(err) => { + bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); + } + }; - // do the impls unify? If not, not disjoint. - let Ok(InferOk { obligations: more_obligations, .. }) = infcx + // Attempt to prove that impl2 applies, given all of the above. + let selcx = &mut SelectionContext::new(&infcx); + let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); + let (impl2_trait_ref, obligations) = + impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); + + // do the impls unify? If not, not disjoint. + let Ok(InferOk { obligations: more_obligations, .. }) = infcx .at(&ObligationCause::dummy(), impl1_env) - .eq(impl1_trait_ref, impl2_trait_ref) - else { - debug!( - "explicit_disjoint: {:?} does not unify with {:?}", - impl1_trait_ref, impl2_trait_ref - ); - return false; - }; + .eq(impl1_trait_ref, impl2_trait_ref) else { + debug!( + "explicit_disjoint: {:?} does not unify with {:?}", + impl1_trait_ref, impl2_trait_ref + ); + return false; + }; - let opt_failing_obligation = obligations - .into_iter() - .chain(more_obligations) - .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); + let opt_failing_obligation = obligations + .into_iter() + .chain(more_obligations) + .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); - if let Some(failing_obligation) = opt_failing_obligation { - debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); - true + if let Some(failing_obligation) = opt_failing_obligation { + debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); + true + } else { + false + } } else { - false + let ty1 = tcx.type_of(impl1_def_id); + let ty2 = tcx.type_of(impl2_def_id); + + let Ok(InferOk { obligations, .. }) = infcx + .at(&ObligationCause::dummy(), impl1_env) + .eq(ty1, ty2) else { + debug!( + "explicit_disjoint: {:?} does not unify with {:?}", + ty1, ty2 + ); + return false; + }; + + let opt_failing_obligation = obligations + .into_iter() + .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); + + if let Some(failing_obligation) = opt_failing_obligation { + debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); + true + } else { + false + } } }) } diff --git a/src/test/ui/coherence/coherence-negative-inherent.rs b/src/test/ui/coherence/coherence-negative-inherent.rs new file mode 100644 index 00000000000..a9e1acc8044 --- /dev/null +++ b/src/test/ui/coherence/coherence-negative-inherent.rs @@ -0,0 +1,22 @@ +// check-pass + +#![feature(negative_impls)] +#![feature(rustc_attrs)] +#![feature(with_negative_coherence)] + +#[rustc_strict_coherence] +trait Foo {} + +impl !Foo for u32 {} + +struct MyStruct(T); + +impl MyStruct { + fn method(&self) {} +} + +impl MyStruct { + fn method(&self) {} +} + +fn main() {} From f3ebafac913472e81c85574fc3d6645070d4e5bf Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 18 Mar 2022 14:07:22 -0300 Subject: [PATCH 2/9] Extract obligations_satisfiable fn --- .../src/traits/coherence.rs | 86 ++++++++++--------- 1 file changed, 44 insertions(+), 42 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 83f54d6dc3d..5e97818a526 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -17,6 +17,7 @@ use crate::traits::{ use rustc_errors::Diagnostic; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::CRATE_HIR_ID; +use rustc_infer::infer::at::ToTrace; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::{util, TraitEngine}; use rustc_middle::traits::specialization_graph::OverlapMode; @@ -26,6 +27,7 @@ use rustc_middle::ty::subst::Subst; use rustc_middle::ty::{self, Ty, TyCtxt}; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; +use std::fmt::Debug; use std::iter; /// Whether we do the orphan check relative to this crate or @@ -327,56 +329,56 @@ fn negative_impl<'cx, 'tcx>( let (impl2_trait_ref, obligations) = impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); - // do the impls unify? If not, not disjoint. - let Ok(InferOk { obligations: more_obligations, .. }) = infcx - .at(&ObligationCause::dummy(), impl1_env) - .eq(impl1_trait_ref, impl2_trait_ref) else { - debug!( - "explicit_disjoint: {:?} does not unify with {:?}", - impl1_trait_ref, impl2_trait_ref - ); - return false; - }; - - let opt_failing_obligation = obligations - .into_iter() - .chain(more_obligations) - .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); - - if let Some(failing_obligation) = opt_failing_obligation { - debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); - true - } else { - false - } + !obligations_satisfiable( + &infcx, + impl1_env, + impl1_def_id, + impl1_trait_ref, + impl2_trait_ref, + obligations, + ) } else { let ty1 = tcx.type_of(impl1_def_id); let ty2 = tcx.type_of(impl2_def_id); - let Ok(InferOk { obligations, .. }) = infcx - .at(&ObligationCause::dummy(), impl1_env) - .eq(ty1, ty2) else { - debug!( - "explicit_disjoint: {:?} does not unify with {:?}", - ty1, ty2 - ); - return false; - }; - - let opt_failing_obligation = obligations - .into_iter() - .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); - - if let Some(failing_obligation) = opt_failing_obligation { - debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); - true - } else { - false - } + !obligations_satisfiable(&infcx, impl1_env, impl1_def_id, ty1, ty2, iter::empty()) } }) } +fn obligations_satisfiable<'cx, 'tcx, T: Debug + ToTrace<'tcx>>( + infcx: &InferCtxt<'cx, 'tcx>, + impl1_env: ty::ParamEnv<'tcx>, + impl1_def_id: DefId, + impl1: T, + impl2: T, + obligations: impl Iterator>, +) -> bool { + // do the impls unify? If not, not disjoint. + let Ok(InferOk { obligations: more_obligations, .. }) = infcx + .at(&ObligationCause::dummy(), impl1_env) + .eq(impl1, impl2) else { + debug!( + "explicit_disjoint: {:?} does not unify with {:?}", + impl1, impl2 + ); + return true; + }; + + let selcx = &mut SelectionContext::new(&infcx); + let opt_failing_obligation = obligations + .into_iter() + .chain(more_obligations) + .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); + + if let Some(failing_obligation) = opt_failing_obligation { + debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); + false + } else { + true + } +} + /// Try to prove that a negative impl exist for the given obligation and their super predicates. #[instrument(level = "debug", skip(selcx))] fn negative_impl_exists<'cx, 'tcx>( From 91b52148ebc16f41632930344bdbe4e093f1970e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 18 Mar 2022 14:13:06 -0300 Subject: [PATCH 3/9] Minor documentation type fixes h/t @pierwill --- compiler/rustc_trait_selection/src/traits/coherence.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 5e97818a526..8698384800c 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -379,7 +379,7 @@ fn obligations_satisfiable<'cx, 'tcx, T: Debug + ToTrace<'tcx>>( } } -/// Try to prove that a negative impl exist for the given obligation and their super predicates. +/// Try to prove that a negative impl exist for the given obligation and its super predicates. #[instrument(level = "debug", skip(selcx))] fn negative_impl_exists<'cx, 'tcx>( selcx: &SelectionContext<'cx, 'tcx>, @@ -393,7 +393,7 @@ fn negative_impl_exists<'cx, 'tcx>( return true; } - // Try to prove a negative obligation exist for super predicates + // Try to prove a negative obligation exists for super predicates for o in util::elaborate_predicates(infcx.tcx, iter::once(o.predicate)) { if resolve_negative_obligation(infcx, param_env, region_context, &o) { return true; From 0cd03c917cd728844048edb41a3d30d414214ad8 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sun, 20 Mar 2022 00:12:03 -0300 Subject: [PATCH 4/9] Extract ImplSubject information --- compiler/rustc_middle/src/hir/mod.rs | 10 ++- compiler/rustc_middle/src/ty/mod.rs | 6 ++ .../src/traits/coherence.rs | 71 ++++++++++--------- 3 files changed, 50 insertions(+), 37 deletions(-) diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 1053f0cefbe..4bc0d3a4d86 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -7,10 +7,10 @@ pub mod nested_filter; pub mod place; use crate::ty::query::Providers; -use crate::ty::TyCtxt; +use crate::ty::{ImplSubject, TyCtxt}; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; -use rustc_hir::def_id::LocalDefId; +use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::*; use rustc_query_system::ich::StableHashingContext; use rustc_span::DUMMY_SP; @@ -54,6 +54,12 @@ impl<'tcx> TyCtxt<'tcx> { pub fn parent_module(self, id: HirId) -> LocalDefId { self.parent_module_from_def_id(id.owner) } + + pub fn impl_header(self, def_id: DefId) -> ImplSubject<'tcx> { + self.impl_trait_ref(def_id) + .map(ImplSubject::Trait) + .unwrap_or_else(|| ImplSubject::Inherent(self.type_of(def_id))) + } } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 83ab761aa55..91d2d64c52e 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -172,6 +172,12 @@ pub struct ImplHeader<'tcx> { pub predicates: Vec>, } +#[derive(Debug)] +pub enum ImplSubject<'tcx> { + Trait(TraitRef<'tcx>), + Inherent(Ty<'tcx>), +} + #[derive( Copy, Clone, diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 8698384800c..a0fbf94c01d 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -24,7 +24,7 @@ use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::ty::fast_reject::{self, TreatParams}; use rustc_middle::ty::fold::TypeFoldable; use rustc_middle::ty::subst::Subst; -use rustc_middle::ty::{self, Ty, TyCtxt}; +use rustc_middle::ty::{self, ImplSubject, Ty, TyCtxt}; use rustc_span::symbol::sym; use rustc_span::DUMMY_SP; use std::fmt::Debug; @@ -307,46 +307,47 @@ fn negative_impl<'cx, 'tcx>( // create a parameter environment corresponding to a (placeholder) instantiation of impl1 let impl1_env = tcx.param_env(impl1_def_id); - if let Some(impl1_trait_ref) = tcx.impl_trait_ref(impl1_def_id) { - // Normalize the trait reference. The WF rules ought to ensure - // that this always succeeds. - let impl1_trait_ref = match traits::fully_normalize( - &infcx, - FulfillmentContext::new(), - ObligationCause::dummy(), - impl1_env, - impl1_trait_ref, - ) { - Ok(impl1_trait_ref) => impl1_trait_ref, - Err(err) => { - bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); - } - }; + match tcx.impl_header(impl1_def_id) { + ImplSubject::Trait(impl1_trait_ref) => { + // Normalize the trait reference. The WF rules ought to ensure + // that this always succeeds. + let impl1_trait_ref = match traits::fully_normalize( + &infcx, + FulfillmentContext::new(), + ObligationCause::dummy(), + impl1_env, + impl1_trait_ref, + ) { + Ok(impl1_trait_ref) => impl1_trait_ref, + Err(err) => { + bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); + } + }; - // Attempt to prove that impl2 applies, given all of the above. - let selcx = &mut SelectionContext::new(&infcx); - let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); - let (impl2_trait_ref, obligations) = - impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); + // Attempt to prove that impl2 applies, given all of the above. + let selcx = &mut SelectionContext::new(&infcx); + let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); + let (impl2_trait_ref, obligations) = + impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); - !obligations_satisfiable( - &infcx, - impl1_env, - impl1_def_id, - impl1_trait_ref, - impl2_trait_ref, - obligations, - ) - } else { - let ty1 = tcx.type_of(impl1_def_id); - let ty2 = tcx.type_of(impl2_def_id); - - !obligations_satisfiable(&infcx, impl1_env, impl1_def_id, ty1, ty2, iter::empty()) + !equate( + &infcx, + impl1_env, + impl1_def_id, + impl1_trait_ref, + impl2_trait_ref, + obligations, + ) + } + ImplSubject::Inherent(ty1) => { + let ty2 = tcx.type_of(impl2_def_id); + !equate(&infcx, impl1_env, impl1_def_id, ty1, ty2, iter::empty()) + } } }) } -fn obligations_satisfiable<'cx, 'tcx, T: Debug + ToTrace<'tcx>>( +fn equate<'cx, 'tcx, T: Debug + ToTrace<'tcx>>( infcx: &InferCtxt<'cx, 'tcx>, impl1_env: ty::ParamEnv<'tcx>, impl1_def_id: DefId, From 64df2ee1eb3fb9eee420b13b48d59fdb5ae2c23d Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Tue, 22 Mar 2022 10:35:17 -0300 Subject: [PATCH 5/9] impl_header -> impl_subject --- compiler/rustc_middle/src/hir/mod.rs | 2 +- compiler/rustc_trait_selection/src/traits/coherence.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/hir/mod.rs b/compiler/rustc_middle/src/hir/mod.rs index 4bc0d3a4d86..a9e22d16ee0 100644 --- a/compiler/rustc_middle/src/hir/mod.rs +++ b/compiler/rustc_middle/src/hir/mod.rs @@ -55,7 +55,7 @@ impl<'tcx> TyCtxt<'tcx> { self.parent_module_from_def_id(id.owner) } - pub fn impl_header(self, def_id: DefId) -> ImplSubject<'tcx> { + pub fn impl_subject(self, def_id: DefId) -> ImplSubject<'tcx> { self.impl_trait_ref(def_id) .map(ImplSubject::Trait) .unwrap_or_else(|| ImplSubject::Inherent(self.type_of(def_id))) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index a0fbf94c01d..559401c0bd1 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -307,7 +307,7 @@ fn negative_impl<'cx, 'tcx>( // create a parameter environment corresponding to a (placeholder) instantiation of impl1 let impl1_env = tcx.param_env(impl1_def_id); - match tcx.impl_header(impl1_def_id) { + match tcx.impl_subject(impl1_def_id) { ImplSubject::Trait(impl1_trait_ref) => { // Normalize the trait reference. The WF rules ought to ensure // that this always succeeds. From 22b311bd826f0a79abfdd2483eed933e86baa31e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 24 Mar 2022 12:27:09 -0300 Subject: [PATCH 6/9] Extract impl_subject_and_oglibations fn and make equate receive subjects --- compiler/rustc_infer/src/infer/at.rs | 25 +++++- compiler/rustc_middle/src/ty/mod.rs | 3 +- compiler/rustc_middle/src/ty/relate.rs | 26 +++++- .../src/traits/coherence.rs | 87 +++++++++---------- 4 files changed, 94 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_infer/src/infer/at.rs b/compiler/rustc_infer/src/infer/at.rs index 94991fdb201..ff8082840e1 100644 --- a/compiler/rustc_infer/src/infer/at.rs +++ b/compiler/rustc_infer/src/infer/at.rs @@ -28,7 +28,7 @@ use super::*; use rustc_middle::ty::relate::{Relate, TypeRelation}; -use rustc_middle::ty::Const; +use rustc_middle::ty::{Const, ImplSubject}; pub struct At<'a, 'tcx> { pub infcx: &'a InferCtxt<'a, 'tcx>, @@ -272,6 +272,29 @@ impl<'a, 'tcx> Trace<'a, 'tcx> { } } +impl<'tcx> ToTrace<'tcx> for ImplSubject<'tcx> { + fn to_trace( + tcx: TyCtxt<'tcx>, + cause: &ObligationCause<'tcx>, + a_is_expected: bool, + a: Self, + b: Self, + ) -> TypeTrace<'tcx> { + match (a, b) { + (ImplSubject::Trait(trait_ref_a), ImplSubject::Trait(trait_ref_b)) => { + ToTrace::to_trace(tcx, cause, a_is_expected, trait_ref_a, trait_ref_b) + } + (ImplSubject::Inherent(ty_a), ImplSubject::Inherent(ty_b)) => { + ToTrace::to_trace(tcx, cause, a_is_expected, ty_a, ty_b) + } + (ImplSubject::Trait(_), ImplSubject::Inherent(_)) + | (ImplSubject::Inherent(_), ImplSubject::Trait(_)) => { + bug!("can not trace TraitRef and Ty"); + } + } + } +} + impl<'tcx> ToTrace<'tcx> for Ty<'tcx> { fn to_trace( _: TyCtxt<'tcx>, diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index 91d2d64c52e..3a0019cf92b 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -44,6 +44,7 @@ use rustc_span::symbol::{kw, Ident, Symbol}; use rustc_span::Span; use rustc_target::abi::Align; +use std::fmt::Debug; use std::hash::Hash; use std::ops::ControlFlow; use std::{fmt, str}; @@ -172,7 +173,7 @@ pub struct ImplHeader<'tcx> { pub predicates: Vec>, } -#[derive(Debug)] +#[derive(Copy, Clone, Debug, TypeFoldable)] pub enum ImplSubject<'tcx> { Trait(TraitRef<'tcx>), Inherent(Ty<'tcx>), diff --git a/compiler/rustc_middle/src/ty/relate.rs b/compiler/rustc_middle/src/ty/relate.rs index 81ee7942c4d..99cd0c789de 100644 --- a/compiler/rustc_middle/src/ty/relate.rs +++ b/compiler/rustc_middle/src/ty/relate.rs @@ -7,7 +7,7 @@ use crate::mir::interpret::{get_slice_bytes, ConstValue, GlobalAlloc, Scalar}; use crate::ty::error::{ExpectedFound, TypeError}; use crate::ty::subst::{GenericArg, GenericArgKind, Subst, SubstsRef}; -use crate::ty::{self, Term, Ty, TyCtxt, TypeFoldable}; +use crate::ty::{self, ImplSubject, Term, Ty, TyCtxt, TypeFoldable}; use rustc_hir as ast; use rustc_hir::def_id::DefId; use rustc_span::DUMMY_SP; @@ -356,6 +356,30 @@ impl<'tcx> Relate<'tcx> for GeneratorWitness<'tcx> { } } +impl<'tcx> Relate<'tcx> for ImplSubject<'tcx> { + #[inline] + fn relate>( + relation: &mut R, + a: ImplSubject<'tcx>, + b: ImplSubject<'tcx>, + ) -> RelateResult<'tcx, ImplSubject<'tcx>> { + match (a, b) { + (ImplSubject::Trait(trait_ref_a), ImplSubject::Trait(trait_ref_b)) => { + let trait_ref = ty::TraitRef::relate(relation, trait_ref_a, trait_ref_b)?; + Ok(ImplSubject::Trait(trait_ref)) + } + (ImplSubject::Inherent(ty_a), ImplSubject::Inherent(ty_b)) => { + let ty = Ty::relate(relation, ty_a, ty_b)?; + Ok(ImplSubject::Inherent(ty)) + } + (ImplSubject::Trait(_), ImplSubject::Inherent(_)) + | (ImplSubject::Inherent(_), ImplSubject::Trait(_)) => { + bug!("can not relate TraitRef and Ty"); + } + } + } +} + impl<'tcx> Relate<'tcx> for Ty<'tcx> { #[inline] fn relate>( diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 559401c0bd1..7ac6c083076 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -17,7 +17,6 @@ use crate::traits::{ use rustc_errors::Diagnostic; use rustc_hir::def_id::{DefId, LOCAL_CRATE}; use rustc_hir::CRATE_HIR_ID; -use rustc_infer::infer::at::ToTrace; use rustc_infer::infer::{InferCtxt, TyCtxtInferExt}; use rustc_infer::traits::{util, TraitEngine}; use rustc_middle::traits::specialization_graph::OverlapMode; @@ -305,72 +304,72 @@ fn negative_impl<'cx, 'tcx>( // Create an infcx, taking the predicates of impl1 as assumptions: tcx.infer_ctxt().enter(|infcx| { // create a parameter environment corresponding to a (placeholder) instantiation of impl1 - let impl1_env = tcx.param_env(impl1_def_id); - - match tcx.impl_subject(impl1_def_id) { + let impl_env = tcx.param_env(impl1_def_id); + let subject1 = match tcx.impl_subject(impl1_def_id) { ImplSubject::Trait(impl1_trait_ref) => { - // Normalize the trait reference. The WF rules ought to ensure - // that this always succeeds. - let impl1_trait_ref = match traits::fully_normalize( + match traits::fully_normalize( &infcx, FulfillmentContext::new(), ObligationCause::dummy(), - impl1_env, + impl_env, impl1_trait_ref, ) { - Ok(impl1_trait_ref) => impl1_trait_ref, + Ok(impl1_trait_ref) => ImplSubject::Trait(impl1_trait_ref), Err(err) => { bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); } - }; - - // Attempt to prove that impl2 applies, given all of the above. - let selcx = &mut SelectionContext::new(&infcx); - let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); - let (impl2_trait_ref, obligations) = - impl_trait_ref_and_oblig(selcx, impl1_env, impl2_def_id, impl2_substs); - - !equate( - &infcx, - impl1_env, - impl1_def_id, - impl1_trait_ref, - impl2_trait_ref, - obligations, - ) + } } - ImplSubject::Inherent(ty1) => { - let ty2 = tcx.type_of(impl2_def_id); - !equate(&infcx, impl1_env, impl1_def_id, ty1, ty2, iter::empty()) - } - } + subject @ ImplSubject::Inherent(_) => subject, + }; + + let (subject2, obligations) = + impl_subject_and_obligations(&infcx, impl_env, subject1, impl2_def_id); + + !equate(&infcx, impl_env, impl1_def_id, subject1, subject2, obligations) }) } -fn equate<'cx, 'tcx, T: Debug + ToTrace<'tcx>>( +fn impl_subject_and_obligations<'cx, 'tcx>( infcx: &InferCtxt<'cx, 'tcx>, - impl1_env: ty::ParamEnv<'tcx>, + impl_env: ty::ParamEnv<'tcx>, + subject1: ImplSubject<'tcx>, + impl2_def_id: DefId, +) -> (ImplSubject<'tcx>, Box> + 'tcx>) { + if let ImplSubject::Trait(_) = subject1 { + // Attempt to prove that impl2 applies, given all of the above. + let selcx = &mut SelectionContext::new(&infcx); + let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); + let (impl2_trait_ref, obligations) = + impl_trait_ref_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs); + + (ImplSubject::Trait(impl2_trait_ref), Box::new(obligations)) + } else { + (infcx.tcx.impl_subject(impl2_def_id), Box::new(iter::empty())) + } +} + +fn equate<'cx, 'tcx>( + infcx: &InferCtxt<'cx, 'tcx>, + impl_env: ty::ParamEnv<'tcx>, impl1_def_id: DefId, - impl1: T, - impl2: T, + subject1: ImplSubject<'tcx>, + subject2: ImplSubject<'tcx>, obligations: impl Iterator>, ) -> bool { // do the impls unify? If not, not disjoint. - let Ok(InferOk { obligations: more_obligations, .. }) = infcx - .at(&ObligationCause::dummy(), impl1_env) - .eq(impl1, impl2) else { - debug!( - "explicit_disjoint: {:?} does not unify with {:?}", - impl1, impl2 - ); - return true; - }; + let Ok(InferOk { obligations: more_obligations, .. }) = + infcx.at(&ObligationCause::dummy(), impl_env).eq(subject1, subject2) + else { + debug!("explicit_disjoint: {:?} does not unify with {:?}", subject1, subject2); + return true; + }; let selcx = &mut SelectionContext::new(&infcx); let opt_failing_obligation = obligations .into_iter() .chain(more_obligations) - .find(|o| negative_impl_exists(selcx, impl1_env, impl1_def_id, o)); + .find(|o| negative_impl_exists(selcx, impl_env, impl1_def_id, o)); if let Some(failing_obligation) = opt_failing_obligation { debug!("overlap: obligation unsatisfiable {:?}", failing_obligation); From f4bd1e14bd40833576e770317912e1a67fc962f4 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 24 Mar 2022 14:17:54 -0300 Subject: [PATCH 7/9] Normalize both trait and inherent --- .../src/traits/coherence.rs | 25 +++++++------------ 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index 7ac6c083076..f3385df513f 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -305,22 +305,15 @@ fn negative_impl<'cx, 'tcx>( tcx.infer_ctxt().enter(|infcx| { // create a parameter environment corresponding to a (placeholder) instantiation of impl1 let impl_env = tcx.param_env(impl1_def_id); - let subject1 = match tcx.impl_subject(impl1_def_id) { - ImplSubject::Trait(impl1_trait_ref) => { - match traits::fully_normalize( - &infcx, - FulfillmentContext::new(), - ObligationCause::dummy(), - impl_env, - impl1_trait_ref, - ) { - Ok(impl1_trait_ref) => ImplSubject::Trait(impl1_trait_ref), - Err(err) => { - bug!("failed to fully normalize {:?}: {:?}", impl1_trait_ref, err); - } - } - } - subject @ ImplSubject::Inherent(_) => subject, + let subject1 = match traits::fully_normalize( + &infcx, + FulfillmentContext::new(), + ObligationCause::dummy(), + impl_env, + tcx.impl_subject(impl1_def_id), + ) { + Ok(s) => s, + Err(err) => bug!("failed to fully normalize {:?}: {:?}", impl1_def_id, err), }; let (subject2, obligations) = From d96faef9138d6618e36fec1919e1c9305fefa96c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 24 Mar 2022 17:47:10 -0300 Subject: [PATCH 8/9] Where bounds are checked on inherent impls --- .../src/traits/coherence.rs | 10 +++++-- .../rustc_trait_selection/src/traits/util.rs | 28 +++++++++++++++++++ ...oherence-negative-inherent-where-bounds.rs | 25 +++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 src/test/ui/coherence/coherence-negative-inherent-where-bounds.rs diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index f3385df513f..ed9ddb51834 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -7,7 +7,7 @@ use crate::infer::outlives::env::OutlivesEnvironment; use crate::infer::{CombinedSnapshot, InferOk, RegionckMode}; use crate::traits::select::IntercrateAmbiguityCause; -use crate::traits::util::impl_trait_ref_and_oblig; +use crate::traits::util::{impl_trait_ref_and_oblig, inherent_impl_and_oblig}; use crate::traits::SkipLeakCheck; use crate::traits::{ self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation, @@ -338,7 +338,13 @@ fn impl_subject_and_obligations<'cx, 'tcx>( (ImplSubject::Trait(impl2_trait_ref), Box::new(obligations)) } else { - (infcx.tcx.impl_subject(impl2_def_id), Box::new(iter::empty())) + // Attempt to prove that impl2 applies, given all of the above. + let selcx = &mut SelectionContext::new(&infcx); + let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); + let (impl2_ty, obligations) = + inherent_impl_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs); + + (ImplSubject::Inherent(impl2_ty), Box::new(obligations)) } } diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index f800e7a1402..7b64deac99d 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -218,6 +218,34 @@ pub fn impl_trait_ref_and_oblig<'a, 'tcx>( (impl_trait_ref, impl_obligations) } +/// Instantiate all bound parameters of the impl with the given substs, +/// returning the resulting trait ref and all obligations that arise. +/// The obligations are closed under normalization. +pub fn inherent_impl_and_oblig<'a, 'tcx>( + selcx: &mut SelectionContext<'a, 'tcx>, + param_env: ty::ParamEnv<'tcx>, + impl_def_id: DefId, + impl_substs: SubstsRef<'tcx>, +) -> (Ty<'tcx>, impl Iterator>) { + let ty = selcx.tcx().type_of(impl_def_id); + let ty = ty.subst(selcx.tcx(), impl_substs); + let Normalized { value: ty, obligations: normalization_obligations1 } = + super::normalize(selcx, param_env, ObligationCause::dummy(), ty); + + let predicates = selcx.tcx().predicates_of(impl_def_id); + let predicates = predicates.instantiate(selcx.tcx(), impl_substs); + let Normalized { value: predicates, obligations: normalization_obligations2 } = + super::normalize(selcx, param_env, ObligationCause::dummy(), predicates); + let impl_obligations = + predicates_for_generics(ObligationCause::dummy(), 0, param_env, predicates); + + let impl_obligations = impl_obligations + .chain(normalization_obligations1.into_iter()) + .chain(normalization_obligations2.into_iter()); + + (ty, impl_obligations) +} + pub fn predicates_for_generics<'tcx>( cause: ObligationCause<'tcx>, recursion_depth: usize, diff --git a/src/test/ui/coherence/coherence-negative-inherent-where-bounds.rs b/src/test/ui/coherence/coherence-negative-inherent-where-bounds.rs new file mode 100644 index 00000000000..39ccaa6ac35 --- /dev/null +++ b/src/test/ui/coherence/coherence-negative-inherent-where-bounds.rs @@ -0,0 +1,25 @@ +// check-pass + +#![feature(negative_impls)] +#![feature(rustc_attrs)] +#![feature(with_negative_coherence)] + +trait Foo {} + +impl !Foo for u32 {} + +#[rustc_strict_coherence] +struct MyStruct(T); + +impl MyStruct { + fn method(&self) {} +} + +impl MyStruct +where + T: Foo, +{ + fn method(&self) {} +} + +fn main() {} From 42e986f77ba350f9cb28958d9ef19af06007ac1c Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Thu, 24 Mar 2022 18:41:30 -0300 Subject: [PATCH 9/9] Implement impl_subject_and_oblig instead of repeating the impls --- .../src/traits/coherence.rs | 32 ++----------- .../src/traits/specialize/mod.rs | 20 ++++---- .../rustc_trait_selection/src/traits/util.rs | 48 ++++--------------- 3 files changed, 26 insertions(+), 74 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/coherence.rs b/compiler/rustc_trait_selection/src/traits/coherence.rs index ed9ddb51834..ec4f8084e21 100644 --- a/compiler/rustc_trait_selection/src/traits/coherence.rs +++ b/compiler/rustc_trait_selection/src/traits/coherence.rs @@ -7,7 +7,7 @@ use crate::infer::outlives::env::OutlivesEnvironment; use crate::infer::{CombinedSnapshot, InferOk, RegionckMode}; use crate::traits::select::IntercrateAmbiguityCause; -use crate::traits::util::{impl_trait_ref_and_oblig, inherent_impl_and_oblig}; +use crate::traits::util::impl_subject_and_oblig; use crate::traits::SkipLeakCheck; use crate::traits::{ self, FulfillmentContext, Normalized, Obligation, ObligationCause, PredicateObligation, @@ -316,38 +316,16 @@ fn negative_impl<'cx, 'tcx>( Err(err) => bug!("failed to fully normalize {:?}: {:?}", impl1_def_id, err), }; + // Attempt to prove that impl2 applies, given all of the above. + 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_obligations(&infcx, impl_env, subject1, impl2_def_id); + impl_subject_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs); !equate(&infcx, impl_env, impl1_def_id, subject1, subject2, obligations) }) } -fn impl_subject_and_obligations<'cx, 'tcx>( - infcx: &InferCtxt<'cx, 'tcx>, - impl_env: ty::ParamEnv<'tcx>, - subject1: ImplSubject<'tcx>, - impl2_def_id: DefId, -) -> (ImplSubject<'tcx>, Box> + 'tcx>) { - if let ImplSubject::Trait(_) = subject1 { - // Attempt to prove that impl2 applies, given all of the above. - let selcx = &mut SelectionContext::new(&infcx); - let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); - let (impl2_trait_ref, obligations) = - impl_trait_ref_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs); - - (ImplSubject::Trait(impl2_trait_ref), Box::new(obligations)) - } else { - // Attempt to prove that impl2 applies, given all of the above. - let selcx = &mut SelectionContext::new(&infcx); - let impl2_substs = infcx.fresh_substs_for_item(DUMMY_SP, impl2_def_id); - let (impl2_ty, obligations) = - inherent_impl_and_oblig(selcx, impl_env, impl2_def_id, impl2_substs); - - (ImplSubject::Inherent(impl2_ty), Box::new(obligations)) - } -} - fn equate<'cx, 'tcx>( infcx: &InferCtxt<'cx, 'tcx>, impl_env: ty::ParamEnv<'tcx>, diff --git a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs index 79471065ccc..328e0d2e0e9 100644 --- a/compiler/rustc_trait_selection/src/traits/specialize/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/specialize/mod.rs @@ -20,12 +20,12 @@ use rustc_errors::{struct_span_err, EmissionGuarantee}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_middle::lint::LintDiagnosticBuilder; use rustc_middle::ty::subst::{InternalSubsts, Subst, SubstsRef}; -use rustc_middle::ty::{self, TyCtxt}; +use rustc_middle::ty::{self, ImplSubject, TyCtxt}; use rustc_session::lint::builtin::COHERENCE_LEAK_CHECK; use rustc_session::lint::builtin::ORDER_DEPENDENT_TRAIT_OBJECTS; use rustc_span::{Span, DUMMY_SP}; -use super::util::impl_trait_ref_and_oblig; +use super::util; use super::{FulfillmentContext, SelectionContext}; /// Information pertinent to an overlapping impl error. @@ -186,18 +186,20 @@ fn fulfill_implication<'a, 'tcx>( param_env, source_trait_ref, target_impl ); + 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_ref, obligations) = - impl_trait_ref_and_oblig(selcx, param_env, target_impl, target_substs); + let (target_trait, obligations) = + util::impl_subject_and_oblig(selcx, param_env, target_impl, target_substs); // do the impls unify? If not, no specialization. let Ok(InferOk { obligations: more_obligations, .. }) = - infcx.at(&ObligationCause::dummy(), param_env).eq(source_trait_ref, target_trait_ref) + infcx.at(&ObligationCause::dummy(), param_env).eq(source_trait, target_trait) else { debug!( "fulfill_implication: {:?} does not unify with {:?}", - source_trait_ref, target_trait_ref + source_trait, target_trait ); return Err(()); }; @@ -225,7 +227,7 @@ fn fulfill_implication<'a, 'tcx>( [] => { debug!( "fulfill_implication: an impl for {:?} specializes {:?}", - source_trait_ref, target_trait_ref + source_trait, target_trait ); // Now resolve the *substitution* we built for the target earlier, replacing @@ -237,8 +239,8 @@ fn fulfill_implication<'a, 'tcx>( debug!( "fulfill_implication: for impls on {:?} and {:?}, \ could not fulfill: {:?} given {:?}", - source_trait_ref, - target_trait_ref, + source_trait, + target_trait, errors, param_env.caller_bounds() ); diff --git a/compiler/rustc_trait_selection/src/traits/util.rs b/compiler/rustc_trait_selection/src/traits/util.rs index 7b64deac99d..7543d1f9a7b 100644 --- a/compiler/rustc_trait_selection/src/traits/util.rs +++ b/compiler/rustc_trait_selection/src/traits/util.rs @@ -6,7 +6,7 @@ use smallvec::SmallVec; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def_id::DefId; use rustc_middle::ty::subst::{GenericArg, Subst, SubstsRef}; -use rustc_middle::ty::{self, ToPredicate, Ty, TyCtxt, TypeFoldable}; +use rustc_middle::ty::{self, ImplSubject, ToPredicate, Ty, TyCtxt, TypeFoldable}; use super::{Normalized, Obligation, ObligationCause, PredicateObligation, SelectionContext}; pub use rustc_infer::traits::{self, util::*}; @@ -190,19 +190,19 @@ impl Iterator for SupertraitDefIds<'_> { // Other /////////////////////////////////////////////////////////////////////////// -/// Instantiate all bound parameters of the impl with the given substs, -/// returning the resulting trait ref and all obligations that arise. +/// Instantiate all bound parameters of the impl subject with the given substs, +/// returning the resulting subject and all obligations that arise. /// The obligations are closed under normalization. -pub fn impl_trait_ref_and_oblig<'a, 'tcx>( +pub fn impl_subject_and_oblig<'a, 'tcx>( selcx: &mut SelectionContext<'a, 'tcx>, param_env: ty::ParamEnv<'tcx>, impl_def_id: DefId, impl_substs: SubstsRef<'tcx>, -) -> (ty::TraitRef<'tcx>, impl Iterator>) { - let impl_trait_ref = selcx.tcx().impl_trait_ref(impl_def_id).unwrap(); - let impl_trait_ref = impl_trait_ref.subst(selcx.tcx(), impl_substs); - let Normalized { value: impl_trait_ref, obligations: normalization_obligations1 } = - super::normalize(selcx, param_env, ObligationCause::dummy(), impl_trait_ref); +) -> (ImplSubject<'tcx>, impl Iterator>) { + let subject = selcx.tcx().impl_subject(impl_def_id); + let subject = subject.subst(selcx.tcx(), impl_substs); + let Normalized { value: subject, obligations: normalization_obligations1 } = + super::normalize(selcx, param_env, ObligationCause::dummy(), subject); let predicates = selcx.tcx().predicates_of(impl_def_id); let predicates = predicates.instantiate(selcx.tcx(), impl_substs); @@ -215,35 +215,7 @@ pub fn impl_trait_ref_and_oblig<'a, 'tcx>( .chain(normalization_obligations1.into_iter()) .chain(normalization_obligations2.into_iter()); - (impl_trait_ref, impl_obligations) -} - -/// Instantiate all bound parameters of the impl with the given substs, -/// returning the resulting trait ref and all obligations that arise. -/// The obligations are closed under normalization. -pub fn inherent_impl_and_oblig<'a, 'tcx>( - selcx: &mut SelectionContext<'a, 'tcx>, - param_env: ty::ParamEnv<'tcx>, - impl_def_id: DefId, - impl_substs: SubstsRef<'tcx>, -) -> (Ty<'tcx>, impl Iterator>) { - let ty = selcx.tcx().type_of(impl_def_id); - let ty = ty.subst(selcx.tcx(), impl_substs); - let Normalized { value: ty, obligations: normalization_obligations1 } = - super::normalize(selcx, param_env, ObligationCause::dummy(), ty); - - let predicates = selcx.tcx().predicates_of(impl_def_id); - let predicates = predicates.instantiate(selcx.tcx(), impl_substs); - let Normalized { value: predicates, obligations: normalization_obligations2 } = - super::normalize(selcx, param_env, ObligationCause::dummy(), predicates); - let impl_obligations = - predicates_for_generics(ObligationCause::dummy(), 0, param_env, predicates); - - let impl_obligations = impl_obligations - .chain(normalization_obligations1.into_iter()) - .chain(normalization_obligations2.into_iter()); - - (ty, impl_obligations) + (subject, impl_obligations) } pub fn predicates_for_generics<'tcx>(