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

Implement coherence checks for negative trait impls #90104

Merged
merged 22 commits into from
Oct 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6975afd
Add polarity to TraitPredicate
spastorino Oct 11, 2021
8b0bfb0
Consider negative polarity on overlap check
spastorino Oct 12, 2021
ab17068
Consider negative polarity on trait selection
spastorino Oct 13, 2021
511076a
Test that if we promise to not impl what would overlap it doesn't act…
spastorino Oct 11, 2021
da8873e
Only assemble_candidates_from_impls for polarity Negative
spastorino Oct 14, 2021
85c8fd9
Make EvaluationCache consider polarity as cache's key
spastorino Oct 14, 2021
89a419c
Filter out Negative impls on intercrate mode's ambiguous reasoning
spastorino Oct 15, 2021
6ae1d68
Use predicate_must_hold_modulo_regions
spastorino Oct 16, 2021
7568632
Filter candidates when goal and impl polarity doesn't match
spastorino Oct 20, 2021
68d444f
Add TraitObligation::polarity() for better encapsulation
spastorino Oct 20, 2021
5a72753
Fix allow_negative_impls logic
spastorino Oct 20, 2021
7829d9d
Document overlap check filter
spastorino Oct 22, 2021
c4c76a4
Document flip polarity
spastorino Oct 22, 2021
5b5a2e6
Move const filter to filter_impls
spastorino Oct 22, 2021
b03a0df
Fix debug method name
spastorino Oct 22, 2021
2e9fb8b
Fix filter_impls comment
spastorino Oct 22, 2021
74454c4
Add comment about the only way to prove NotImplemented here
spastorino Oct 22, 2021
da79fa9
Add rustc_strict_coherence attribute and use it to check overlap
spastorino Oct 22, 2021
132409f
Assemple trait alias candidates for negative polarity
spastorino Oct 22, 2021
9e26413
Be sure that we do not allow too much
spastorino Oct 22, 2021
9534186
Hide negative coherence checks under negative_impls feature flag
spastorino Oct 22, 2021
3287f72
Avoid code duplication by extracting checks into fns
spastorino Oct 23, 2021
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
1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/type_check/canonical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
Some(ty::Binder::dummy(ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: ty::ImplPolarity::Positive,
}))),
locations,
category,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,7 @@ impl Visitor<'tcx> for Checker<'mir, 'tcx> {
Binder::dummy(TraitPredicate {
trait_ref,
constness: ty::BoundConstness::ConstIfConst,
polarity: ty::ImplPolarity::Positive,
}),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ impl Qualif for NeedsNonConstDrop {
ty::Binder::dummy(ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::ConstIfConst,
polarity: ty::ImplPolarity::Positive,
}),
);

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_feature/src/builtin_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[
rustc_attr!(TEST, rustc_outlives, Normal, template!(Word)),
rustc_attr!(TEST, rustc_capture_analysis, Normal, template!(Word)),
rustc_attr!(TEST, rustc_insignificant_dtor, Normal, template!(Word)),
rustc_attr!(TEST, rustc_strict_coherence, Normal, template!(Word)),
rustc_attr!(TEST, rustc_variance, Normal, template!(Word)),
rustc_attr!(TEST, rustc_layout, Normal, template!(List: "field1, field2, ...")),
rustc_attr!(TEST, rustc_regions, Normal, template!(Word)),
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_infer/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub mod util;

use rustc_hir as hir;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::{self, Const, Ty};
use rustc_middle::ty::{self, Const, Ty, TyCtxt};
use rustc_span::Span;

pub use self::FulfillmentErrorCode::*;
Expand Down Expand Up @@ -55,6 +55,20 @@ pub struct Obligation<'tcx, T> {
pub type PredicateObligation<'tcx> = Obligation<'tcx, ty::Predicate<'tcx>>;
pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>;

impl PredicateObligation<'tcx> {
/// Flips the polarity of the inner predicate.
///
/// Given `T: Trait` predicate it returns `T: !Trait` and given `T: !Trait` returns `T: Trait`.
pub fn flip_polarity(&self, tcx: TyCtxt<'tcx>) -> Option<PredicateObligation<'tcx>> {
Some(PredicateObligation {
cause: self.cause.clone(),
param_env: self.param_env,
predicate: self.predicate.flip_polarity(tcx)?,
recursion_depth: self.recursion_depth,
})
}
}

// `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
static_assert_size!(PredicateObligation<'_>, 32);
Expand Down Expand Up @@ -129,6 +143,10 @@ impl<'tcx> FulfillmentError<'tcx> {
}

impl<'tcx> TraitObligation<'tcx> {
pub fn polarity(&self) -> ty::ImplPolarity {
self.predicate.skip_binder().polarity
}

pub fn self_ty(&self) -> ty::Binder<'tcx, Ty<'tcx>> {
self.predicate.map_bound(|p| p.self_ty())
}
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_middle/src/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ use rustc_hir::def_id::DefId;
use rustc_query_system::cache::Cache;

pub type SelectionCache<'tcx> = Cache<
ty::ConstnessAnd<ty::ParamEnvAnd<'tcx, ty::TraitRef<'tcx>>>,
(ty::ConstnessAnd<ty::ParamEnvAnd<'tcx, ty::TraitRef<'tcx>>>, ty::ImplPolarity),
SelectionResult<'tcx, SelectionCandidate<'tcx>>,
>;

pub type EvaluationCache<'tcx> =
Cache<ty::ParamEnvAnd<'tcx, ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>>, EvaluationResult>;
pub type EvaluationCache<'tcx> = Cache<
(ty::ParamEnvAnd<'tcx, ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>>, ty::ImplPolarity),
EvaluationResult,
>;

/// The selection process begins by considering all impls, where
/// clauses, and so forth that might resolve an obligation. Sometimes
Expand Down Expand Up @@ -101,7 +103,7 @@ pub enum SelectionCandidate<'tcx> {
/// `false` if there are no *further* obligations.
has_nested: bool,
},
ParamCandidate(ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>),
ParamCandidate((ty::ConstnessAnd<ty::PolyTraitRef<'tcx>>, ty::ImplPolarity)),
ImplCandidate(DefId),
AutoImplCandidate(DefId),

Expand Down
11 changes: 7 additions & 4 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl<T> ExpectedFound<T> {
pub enum TypeError<'tcx> {
Mismatch,
ConstnessMismatch(ExpectedFound<ty::BoundConstness>),
PolarityMismatch(ExpectedFound<ty::ImplPolarity>),
UnsafetyMismatch(ExpectedFound<hir::Unsafety>),
AbiMismatch(ExpectedFound<abi::Abi>),
Mutability,
Expand Down Expand Up @@ -104,6 +105,9 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
ConstnessMismatch(values) => {
write!(f, "expected {} bound, found {} bound", values.expected, values.found)
}
PolarityMismatch(values) => {
write!(f, "expected {} polarity, found {} polarity", values.expected, values.found)
}
UnsafetyMismatch(values) => {
write!(f, "expected {} fn, found {} fn", values.expected, values.found)
}
Expand Down Expand Up @@ -212,10 +216,9 @@ impl<'tcx> TypeError<'tcx> {
use self::TypeError::*;
match self {
CyclicTy(_) | CyclicConst(_) | UnsafetyMismatch(_) | ConstnessMismatch(_)
| Mismatch | AbiMismatch(_) | FixedArraySize(_) | ArgumentSorts(..) | Sorts(_)
| IntMismatch(_) | FloatMismatch(_) | VariadicMismatch(_) | TargetFeatureCast(_) => {
false
}
| PolarityMismatch(_) | Mismatch | AbiMismatch(_) | FixedArraySize(_)
| ArgumentSorts(..) | Sorts(_) | IntMismatch(_) | FloatMismatch(_)
| VariadicMismatch(_) | TargetFeatureCast(_) => false,

Mutability
| ArgumentMutability(_)
Expand Down
65 changes: 63 additions & 2 deletions compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,18 @@ pub struct ImplHeader<'tcx> {
pub predicates: Vec<Predicate<'tcx>>,
}

#[derive(Copy, Clone, PartialEq, TyEncodable, TyDecodable, HashStable, Debug)]
#[derive(
Copy,
Clone,
PartialEq,
Eq,
Hash,
TyEncodable,
TyDecodable,
HashStable,
Debug,
TypeFoldable
)]
pub enum ImplPolarity {
/// `impl Trait for Type`
Positive,
Expand All @@ -178,6 +189,27 @@ pub enum ImplPolarity {
Reservation,
}

impl ImplPolarity {
/// Flips polarity by turning `Positive` into `Negative` and `Negative` into `Positive`.
pub fn flip(&self) -> Option<ImplPolarity> {
match self {
ImplPolarity::Positive => Some(ImplPolarity::Negative),
ImplPolarity::Negative => Some(ImplPolarity::Positive),
ImplPolarity::Reservation => None,
}
}
}

impl fmt::Display for ImplPolarity {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Positive => f.write_str("positive"),
Self::Negative => f.write_str("negative"),
Self::Reservation => f.write_str("reservation"),
}
}
}

#[derive(Clone, Debug, PartialEq, Eq, Copy, Hash, TyEncodable, TyDecodable, HashStable)]
pub enum Visibility {
/// Visible everywhere (including in other crates).
Expand Down Expand Up @@ -460,6 +492,29 @@ impl<'tcx> Predicate<'tcx> {
pub fn kind(self) -> Binder<'tcx, PredicateKind<'tcx>> {
self.inner.kind
}

/// Flips the polarity of a Predicate.
///
/// Given `T: Trait` predicate it returns `T: !Trait` and given `T: !Trait` returns `T: Trait`.
pub fn flip_polarity(&self, tcx: TyCtxt<'tcx>) -> Option<Predicate<'tcx>> {
let kind = self
.inner
.kind
.map_bound(|kind| match kind {
PredicateKind::Trait(TraitPredicate { trait_ref, constness, polarity }) => {
Some(PredicateKind::Trait(TraitPredicate {
trait_ref,
constness,
polarity: polarity.flip()?,
}))
}

_ => None,
})
.transpose()?;

Some(tcx.mk_predicate(kind))
}
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for Predicate<'tcx> {
Expand Down Expand Up @@ -655,6 +710,8 @@ pub struct TraitPredicate<'tcx> {
pub trait_ref: TraitRef<'tcx>,

pub constness: BoundConstness,

pub polarity: ImplPolarity,
}

pub type PolyTraitPredicate<'tcx> = ty::Binder<'tcx, TraitPredicate<'tcx>>;
Expand Down Expand Up @@ -789,7 +846,11 @@ impl<'tcx> ToPredicate<'tcx> for ConstnessAnd<PolyTraitRef<'tcx>> {
fn to_predicate(self, tcx: TyCtxt<'tcx>) -> Predicate<'tcx> {
self.value
.map_bound(|trait_ref| {
PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: self.constness })
PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: self.constness,
polarity: ty::ImplPolarity::Positive,
})
})
.to_predicate(tcx)
}
Expand Down
15 changes: 15 additions & 0 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -797,6 +797,20 @@ impl<'tcx> Relate<'tcx> for GenericArg<'tcx> {
}
}

impl<'tcx> Relate<'tcx> for ty::ImplPolarity {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
a: ty::ImplPolarity,
b: ty::ImplPolarity,
) -> RelateResult<'tcx, ty::ImplPolarity> {
if a != b {
Err(TypeError::PolarityMismatch(expected_found(relation, a, b)))
} else {
Ok(a)
}
}
}

impl<'tcx> Relate<'tcx> for ty::TraitPredicate<'tcx> {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
Expand All @@ -806,6 +820,7 @@ impl<'tcx> Relate<'tcx> for ty::TraitPredicate<'tcx> {
Ok(ty::TraitPredicate {
trait_ref: relation.relate(a.trait_ref, b.trait_ref)?,
constness: relation.relate(a.constness, b.constness)?,
polarity: relation.relate(a.polarity, b.polarity)?,
})
}
}
Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl fmt::Debug for ty::TraitPredicate<'tcx> {
if let ty::BoundConstness::ConstIfConst = self.constness {
write!(f, "~const ")?;
}
write!(f, "TraitPredicate({:?})", self.trait_ref)
write!(f, "TraitPredicate({:?}, polarity:{:?})", self.trait_ref, self.polarity)
}
}

Expand Down Expand Up @@ -365,8 +365,11 @@ impl<'a, 'tcx> Lift<'tcx> for ty::ExistentialPredicate<'a> {
impl<'a, 'tcx> Lift<'tcx> for ty::TraitPredicate<'a> {
type Lifted = ty::TraitPredicate<'tcx>;
fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option<ty::TraitPredicate<'tcx>> {
tcx.lift(self.trait_ref)
.map(|trait_ref| ty::TraitPredicate { trait_ref, constness: self.constness })
tcx.lift(self.trait_ref).map(|trait_ref| ty::TraitPredicate {
trait_ref,
constness: self.constness,
polarity: self.polarity,
})
}
}

Expand Down Expand Up @@ -591,6 +594,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
Some(match self {
Mismatch => Mismatch,
ConstnessMismatch(x) => ConstnessMismatch(x),
PolarityMismatch(x) => PolarityMismatch(x),
UnsafetyMismatch(x) => UnsafetyMismatch(x),
AbiMismatch(x) => AbiMismatch(x),
Mutability => Mutability,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -882,6 +882,7 @@ impl<'tcx> PolyTraitRef<'tcx> {
self.map_bound(|trait_ref| ty::TraitPredicate {
trait_ref,
constness: ty::BoundConstness::NotConst,
polarity: ty::ImplPolarity::Positive,
})
}
}
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,11 @@ where

fn visit_predicate(&mut self, predicate: ty::Predicate<'tcx>) -> ControlFlow<V::BreakTy> {
match predicate.kind().skip_binder() {
ty::PredicateKind::Trait(ty::TraitPredicate { trait_ref, constness: _ }) => {
self.visit_trait(trait_ref)
}
ty::PredicateKind::Trait(ty::TraitPredicate {
trait_ref,
constness: _,
polarity: _,
}) => self.visit_trait(trait_ref),
ty::PredicateKind::Projection(ty::ProjectionPredicate { projection_ty, ty }) => {
ty.visit_with(self)?;
self.visit_projection_ty(projection_ty)
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,7 @@ symbols! {
rustc_specialization_trait,
rustc_stable,
rustc_std_internal_symbol,
rustc_strict_coherence,
rustc_symbol_name,
rustc_synthetic,
rustc_test_marker,
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_trait_selection/src/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,8 @@ impl AutoTraitFinder<'tcx> {
substs: infcx.tcx.mk_substs_trait(ty, &[]),
},
constness: ty::BoundConstness::NotConst,
// Auto traits are positive
polarity: ty::ImplPolarity::Positive,
}));

let computed_preds = param_env.caller_bounds().iter();
Expand Down
Loading