From debdb8bca82b9702fa08204f016fdd1e33087024 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 1 Oct 2019 05:49:12 -0400 Subject: [PATCH 1/5] extract `Polarity` from `PolarizedTraitRef` also move some methods to `TraitDatum` --- chalk-parse/src/ast.rs | 20 ++++++++----- chalk-parse/src/parser.lalrpop | 5 ++-- chalk-rust-ir/src/lib.rs | 34 +++++++++------------- chalk-solve/src/clauses/program_clauses.rs | 3 +- chalk-solve/src/coherence/orphan.rs | 4 +-- chalk-solve/src/coherence/solve.rs | 10 ++----- chalk-solve/src/wf.rs | 10 +++---- src/db.rs | 7 ++--- src/lowering.rs | 30 +++++++++---------- src/query.rs | 2 +- 10 files changed, 59 insertions(+), 66 deletions(-) diff --git a/chalk-parse/src/ast.rs b/chalk-parse/src/ast.rs index 2f8270fc689..dc1fa32077a 100644 --- a/chalk-parse/src/ast.rs +++ b/chalk-parse/src/ast.rs @@ -128,7 +128,8 @@ impl fmt::Display for Kind { #[derive(Clone, PartialEq, Eq, Debug)] pub struct Impl { pub parameter_kinds: Vec, - pub trait_ref: PolarizedTraitRef, + pub trait_ref: TraitRef, + pub polarity: Polarity, pub where_clauses: Vec, pub assoc_ty_values: Vec, pub impl_type: ImplType, @@ -185,17 +186,20 @@ pub struct TraitRef { } #[derive(Clone, PartialEq, Eq, Debug)] -pub enum PolarizedTraitRef { - Positive(TraitRef), - Negative(TraitRef), +pub enum Polarity { + /// `impl Foo for Bar` + Positive, + + /// `impl !Foo for Bar` + Negative, } -impl PolarizedTraitRef { - pub fn from_bool(polarity: bool, trait_ref: TraitRef) -> PolarizedTraitRef { +impl Polarity { + pub fn from_bool(polarity: bool) -> Polarity { if polarity { - PolarizedTraitRef::Positive(trait_ref) + Polarity::Positive } else { - PolarizedTraitRef::Negative(trait_ref) + Polarity::Negative } } } diff --git a/chalk-parse/src/parser.lalrpop b/chalk-parse/src/parser.lalrpop index e1c6b9a0a1c..fdf4a2712d9 100644 --- a/chalk-parse/src/parser.lalrpop +++ b/chalk-parse/src/parser.lalrpop @@ -136,10 +136,11 @@ Impl: Impl = { args.extend(a); Impl { parameter_kinds: p, - trait_ref: PolarizedTraitRef::from_bool(mark.is_none(), TraitRef { + polarity: Polarity::from_bool(mark.is_none()), + trait_ref: TraitRef { trait_name: t, args: args, - }), + }, where_clauses: w, assoc_ty_values: assoc, impl_type: external.map(|_| ImplType::External).unwrap_or(ImplType::Local), diff --git a/chalk-rust-ir/src/lib.rs b/chalk-rust-ir/src/lib.rs index 954a3f85dad..48cdbd0d8f5 100644 --- a/chalk-rust-ir/src/lib.rs +++ b/chalk-rust-ir/src/lib.rs @@ -18,21 +18,23 @@ pub enum LangItem {} #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ImplDatum { + pub polarity: Polarity, pub binders: Binders, } impl ImplDatum { pub fn is_positive(&self) -> bool { - match self.binders.value.trait_ref { - PolarizedTraitRef::Positive(_) => true, - PolarizedTraitRef::Negative(_) => false, - } + self.polarity.is_positive() + } + + pub fn trait_id(&self) -> TraitId { + self.binders.value.trait_ref.trait_id } } #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct ImplDatumBound { - pub trait_ref: PolarizedTraitRef, + pub trait_ref: TraitRef, pub where_clauses: Vec, pub associated_ty_values: Vec, pub impl_type: ImplType, @@ -400,25 +402,17 @@ pub enum TypeSort { Trait, } -#[derive(Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug)] -pub enum PolarizedTraitRef { - Positive(TraitRef), - Negative(TraitRef), +#[derive(Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, Debug)] +pub enum Polarity { + Positive, + Negative, } -enum_fold!(PolarizedTraitRef[] { Positive(a), Negative(a) }); - -impl PolarizedTraitRef { +impl Polarity { pub fn is_positive(&self) -> bool { match *self { - PolarizedTraitRef::Positive(_) => true, - PolarizedTraitRef::Negative(_) => false, - } - } - - pub fn trait_ref(&self) -> &TraitRef { - match *self { - PolarizedTraitRef::Positive(ref tr) | PolarizedTraitRef::Negative(ref tr) => tr, + Polarity::Positive => true, + Polarity::Negative => false, } } } diff --git a/chalk-solve/src/clauses/program_clauses.rs b/chalk-solve/src/clauses/program_clauses.rs index 98e9fc440e3..4ec153308c3 100644 --- a/chalk-solve/src/clauses/program_clauses.rs +++ b/chalk-solve/src/clauses/program_clauses.rs @@ -32,7 +32,7 @@ impl ToProgramClauses for ImplDatum { clauses.push( self.binders .map_ref(|bound| ProgramClauseImplication { - consequence: bound.trait_ref.trait_ref().clone().cast(), + consequence: bound.trait_ref.clone().cast(), conditions: bound.where_clauses.iter().cloned().casted().collect(), }) .cast(), @@ -94,7 +94,6 @@ impl ToProgramClauses for AssociatedTyValue { .binders .value .trait_ref - .trait_ref() .shifted_in(self.value.len()); let all_parameters: Vec<_> = self diff --git a/chalk-solve/src/coherence/orphan.rs b/chalk-solve/src/coherence/orphan.rs index c7038734eb2..16da0d70c95 100644 --- a/chalk-solve/src/coherence/orphan.rs +++ b/chalk-solve/src/coherence/orphan.rs @@ -26,7 +26,7 @@ pub fn perform_orphan_check( .binders .map_ref(|bound_impl| { // Ignoring the polarization of the impl's polarized trait ref - DomainGoal::LocalImplAllowed(bound_impl.trait_ref.trait_ref().clone()) + DomainGoal::LocalImplAllowed(bound_impl.trait_ref.clone()) }) .cast(); @@ -38,7 +38,7 @@ pub fn perform_orphan_check( debug!("overlaps = {:?}", is_allowed); if !is_allowed { - let trait_id = impl_datum.binders.value.trait_ref.trait_ref().trait_id; + let trait_id = impl_datum.trait_id(); let trait_name = db.type_name(trait_id.into()); Err(CoherenceError::FailedOrphanCheck(trait_name))?; } diff --git a/chalk-solve/src/coherence/solve.rs b/chalk-solve/src/coherence/solve.rs index 9087166ae98..2189761459e 100644 --- a/chalk-solve/src/coherence/solve.rs +++ b/chalk-solve/src/coherence/solve.rs @@ -29,9 +29,7 @@ where let rhs = &self.db.impl_datum(r_id); // Two negative impls never overlap. - if !lhs.binders.value.trait_ref.is_positive() - && !rhs.binders.value.trait_ref.is_positive() - { + if !lhs.is_positive() && !rhs.is_positive() { continue; } @@ -171,9 +169,7 @@ where ); // Negative impls cannot specialize. - if !less_special.binders.value.trait_ref.is_positive() - || !more_special.binders.value.trait_ref.is_positive() - { + if !less_special.is_positive() || !more_special.is_positive() { return false; } @@ -228,5 +224,5 @@ where } fn params(impl_datum: &ImplDatum) -> &[Parameter] { - &impl_datum.binders.value.trait_ref.trait_ref().parameters + &impl_datum.binders.value.trait_ref.parameters } diff --git a/chalk-solve/src/wf.rs b/chalk-solve/src/wf.rs index 726aefd9364..deea34d339a 100644 --- a/chalk-solve/src/wf.rs +++ b/chalk-solve/src/wf.rs @@ -181,10 +181,9 @@ where pub fn verify_trait_impl(&self, impl_id: ImplId) -> Result<(), WfError> { let impl_datum = self.db.impl_datum(impl_id); - let trait_ref = match impl_datum.binders.value.trait_ref { - PolarizedTraitRef::Positive(ref trait_ref) => trait_ref, - _ => return Ok(()), - }; + if !impl_datum.is_positive() { + return Ok(()); + } // We retrieve all the input types of the where clauses appearing on the trait impl, // e.g. in: @@ -214,6 +213,7 @@ where // } // ``` let mut header_input_types = Vec::new(); + let trait_ref = &impl_datum.binders.value.trait_ref; trait_ref.fold(&mut header_input_types); // Associated type values are special because they can be parametric (independently of @@ -338,7 +338,7 @@ where if is_legal { Ok(()) } else { - let trait_ref = impl_datum.binders.value.trait_ref.trait_ref(); + let trait_ref = &impl_datum.binders.value.trait_ref; let name = self.db.type_name(trait_ref.trait_id.into()); Err(WfError::IllFormedTraitImpl(name)) } diff --git a/src/db.rs b/src/db.rs index 01bd5c847a9..228ac69ba87 100644 --- a/src/db.rs +++ b/src/db.rs @@ -93,7 +93,7 @@ impl RustIrDatabase for ChalkDatabase { .impl_data .iter() .filter(|(_, impl_datum)| { - let trait_ref = impl_datum.binders.value.trait_ref.trait_ref(); + let trait_ref = &impl_datum.binders.value.trait_ref; trait_id == trait_ref.trait_id && { assert_eq!(trait_ref.parameters.len(), parameters.len()); <[_] as CouldMatch<[_]>>::could_match(¶meters, &trait_ref.parameters) @@ -109,9 +109,8 @@ impl RustIrDatabase for ChalkDatabase { .impl_data .iter() .filter(|(_, impl_datum)| { - let impl_trait_id = impl_datum.binders.value.trait_ref.trait_ref().trait_id; let impl_type = impl_datum.binders.value.impl_type; - impl_trait_id == trait_id && impl_type == ImplType::Local + impl_datum.trait_id() == trait_id && impl_type == ImplType::Local }) .map(|(&impl_id, _)| impl_id) .collect() @@ -126,7 +125,7 @@ impl RustIrDatabase for ChalkDatabase { .impl_data .values() .any(|impl_datum| { - let impl_trait_ref = impl_datum.binders.value.trait_ref.trait_ref(); + let impl_trait_ref = &impl_datum.binders.value.trait_ref; impl_trait_ref.trait_id == auto_trait_id && match impl_trait_ref.parameters[0].assert_ty_ref() { Ty::Apply(apply) => match apply.name { diff --git a/src/lowering.rs b/src/lowering.rs index 57e50291db9..aec88986192 100644 --- a/src/lowering.rs +++ b/src/lowering.rs @@ -767,20 +767,16 @@ impl LowerQuantifiedInlineBoundVec for [QuantifiedInlineBound] { } } -trait LowerPolarizedTraitRef { - fn lower(&self, env: &Env) -> Fallible; +trait LowerPolarity { + fn lower(&self) -> rust_ir::Polarity; } -impl LowerPolarizedTraitRef for PolarizedTraitRef { - fn lower(&self, env: &Env) -> Fallible { - Ok(match *self { - PolarizedTraitRef::Positive(ref tr) => { - rust_ir::PolarizedTraitRef::Positive(tr.lower(env)?) - } - PolarizedTraitRef::Negative(ref tr) => { - rust_ir::PolarizedTraitRef::Negative(tr.lower(env)?) - } - }) +impl LowerPolarity for Polarity { + fn lower(&self) -> rust_ir::Polarity { + match *self { + Polarity::Positive => rust_ir::Polarity::Positive, + Polarity::Negative => rust_ir::Polarity::Negative, + } } } @@ -943,16 +939,17 @@ trait LowerImpl { impl LowerImpl for Impl { fn lower_impl(&self, empty_env: &Env, impl_id: ImplId) -> Fallible { + let polarity = self.polarity.lower(); let binders = empty_env.in_binders(self.all_parameters(), |env| { let trait_ref = self.trait_ref.lower(env)?; - if !trait_ref.is_positive() && !self.assoc_ty_values.is_empty() { + if !polarity.is_positive() && !self.assoc_ty_values.is_empty() { return Err(format_err!( "negative impls cannot define associated values" )); } - let trait_id = trait_ref.trait_ref().trait_id; + let trait_id = trait_ref.trait_id; let where_clauses = self.lower_where_clauses(&env)?; let associated_ty_values = self .assoc_ty_values @@ -970,7 +967,10 @@ impl LowerImpl for Impl { }) })?; - Ok(rust_ir::ImplDatum { binders: binders }) + Ok(rust_ir::ImplDatum { + polarity, + binders: binders, + }) } } diff --git a/src/query.rs b/src/query.rs index 4e48c4c7b93..84108fd4fa1 100644 --- a/src/query.rs +++ b/src/query.rs @@ -152,7 +152,7 @@ fn environment(db: &impl LoweringDatabase) -> Result, Ch for datum in program.impl_data.values() { // If we encounter a negative impl, do not generate any rule. Negative impls // are currently just there to deactivate default impls for auto traits. - if datum.binders.value.trait_ref.is_positive() { + if datum.is_positive() { datum.to_program_clauses(db, &mut program_clauses); datum .binders From 9438aa40fc29b82db87c408232f99b2ea6b706d3 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 1 Oct 2019 05:50:29 -0400 Subject: [PATCH 2/5] extract `impl_type` from binders --- chalk-rust-ir/src/lib.rs | 2 +- src/db.rs | 3 +-- src/lowering.rs | 8 ++++---- src/program.rs | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/chalk-rust-ir/src/lib.rs b/chalk-rust-ir/src/lib.rs index 48cdbd0d8f5..241bbe4e113 100644 --- a/chalk-rust-ir/src/lib.rs +++ b/chalk-rust-ir/src/lib.rs @@ -20,6 +20,7 @@ pub enum LangItem {} pub struct ImplDatum { pub polarity: Polarity, pub binders: Binders, + pub impl_type: ImplType, } impl ImplDatum { @@ -37,7 +38,6 @@ pub struct ImplDatumBound { pub trait_ref: TraitRef, pub where_clauses: Vec, pub associated_ty_values: Vec, - pub impl_type: ImplType, } #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] diff --git a/src/db.rs b/src/db.rs index 228ac69ba87..121981cbcdf 100644 --- a/src/db.rs +++ b/src/db.rs @@ -109,8 +109,7 @@ impl RustIrDatabase for ChalkDatabase { .impl_data .iter() .filter(|(_, impl_datum)| { - let impl_type = impl_datum.binders.value.impl_type; - impl_datum.trait_id() == trait_id && impl_type == ImplType::Local + impl_datum.trait_id() == trait_id && impl_datum.impl_type == ImplType::Local }) .map(|(&impl_id, _)| impl_id) .collect() diff --git a/src/lowering.rs b/src/lowering.rs index aec88986192..b533fcfbfe0 100644 --- a/src/lowering.rs +++ b/src/lowering.rs @@ -960,16 +960,16 @@ impl LowerImpl for Impl { trait_ref, where_clauses, associated_ty_values, - impl_type: match self.impl_type { - ImplType::Local => rust_ir::ImplType::Local, - ImplType::External => rust_ir::ImplType::External, - }, }) })?; Ok(rust_ir::ImplDatum { polarity, binders: binders, + impl_type: match self.impl_type { + ImplType::Local => rust_ir::ImplType::Local, + ImplType::External => rust_ir::ImplType::External, + }, }) } } diff --git a/src/program.rs b/src/program.rs index ef3b4c32e28..1e878b2d56d 100644 --- a/src/program.rs +++ b/src/program.rs @@ -61,7 +61,7 @@ impl Program { pub(crate) fn local_impl_ids(&self) -> Vec { self.impl_data .iter() - .filter(|(_, impl_datum)| impl_datum.binders.value.impl_type == ImplType::Local) + .filter(|(_, impl_datum)| impl_datum.impl_type == ImplType::Local) .map(|(&impl_id, _)| impl_id) .collect() } From a2b7c848b7415448ecffffcf70719f0fd74507bf Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 1 Oct 2019 05:52:26 -0400 Subject: [PATCH 3/5] extract flag from the "bound" part of a trait These do not reference the type parameters on the trait definition --- chalk-rust-ir/src/lib.rs | 14 +++++++------- chalk-solve/src/clauses/program_clauses.rs | 4 ++-- chalk-solve/src/coherence/solve.rs | 2 +- src/lowering.rs | 18 ++++++++++-------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/chalk-rust-ir/src/lib.rs b/chalk-rust-ir/src/lib.rs index 241bbe4e113..8d4a3e9882d 100644 --- a/chalk-rust-ir/src/lib.rs +++ b/chalk-rust-ir/src/lib.rs @@ -79,15 +79,20 @@ pub struct StructFlags { #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub struct TraitDatum { pub binders: Binders, + + /// "Flags" indicate special kinds of traits, like auto traits. + /// In Rust syntax these are represented in different ways, but in + /// chalk we add annotations like `#[auto]`. + pub flags: TraitFlags, } impl TraitDatum { pub fn is_auto_trait(&self) -> bool { - self.binders.value.flags.auto + self.flags.auto } pub fn is_non_enumerable_trait(&self) -> bool { - self.binders.value.flags.non_enumerable + self.flags.non_enumerable } } @@ -112,11 +117,6 @@ pub struct TraitDatumBound { /// ``` pub where_clauses: Vec, - /// "Flags" indicate special kinds of traits, like auto traits. - /// In Rust syntax these are represented in different ways, but in - /// chalk we add annotations like `#[auto]`. - pub flags: TraitFlags, - /// The id of each associated type defined in the trait. pub associated_ty_ids: Vec, } diff --git a/chalk-solve/src/clauses/program_clauses.rs b/chalk-solve/src/clauses/program_clauses.rs index 4ec153308c3..52d25004aca 100644 --- a/chalk-solve/src/clauses/program_clauses.rs +++ b/chalk-solve/src/clauses/program_clauses.rs @@ -518,7 +518,7 @@ impl ToProgramClauses for TraitDatum { impl_may_exist })); - if !self.binders.value.flags.upstream { + if !self.flags.upstream { let impl_allowed = self .binders .map_ref(|bound_datum| ProgramClauseImplication { @@ -548,7 +548,7 @@ impl ToProgramClauses for TraitDatum { // Fundamental traits can be reasoned about negatively without any ambiguity, so no // need for this rule if the trait is fundamental. - if !self.binders.value.flags.fundamental { + if !self.flags.fundamental { let impl_may_exist = self .binders .map_ref(|bound_datum| ProgramClauseImplication { diff --git a/chalk-solve/src/coherence/solve.rs b/chalk-solve/src/coherence/solve.rs index 2189761459e..38b0f14cdd7 100644 --- a/chalk-solve/src/coherence/solve.rs +++ b/chalk-solve/src/coherence/solve.rs @@ -18,7 +18,7 @@ where ) -> Result<(), CoherenceError> { // Ignore impls for marker traits as they are allowed to overlap. let trait_datum = self.db.trait_datum(self.trait_id); - if trait_datum.binders.value.flags.marker { + if trait_datum.flags.marker { return Ok(()); } diff --git a/src/lowering.rs b/src/lowering.rs index b533fcfbfe0..3f11ce36878 100644 --- a/src/lowering.rs +++ b/src/lowering.rs @@ -1080,18 +1080,20 @@ impl LowerTrait for TraitDefn { Ok(rust_ir::TraitDatumBound { trait_ref: trait_ref, where_clauses: self.lower_where_clauses(env)?, - flags: rust_ir::TraitFlags { - auto: self.flags.auto, - marker: self.flags.marker, - upstream: self.flags.upstream, - fundamental: self.flags.fundamental, - non_enumerable: self.flags.non_enumerable, - }, associated_ty_ids, }) })?; - Ok(rust_ir::TraitDatum { binders: binders }) + Ok(rust_ir::TraitDatum { + binders: binders, + flags: rust_ir::TraitFlags { + auto: self.flags.auto, + marker: self.flags.marker, + upstream: self.flags.upstream, + fundamental: self.flags.fundamental, + non_enumerable: self.flags.non_enumerable, + }, + }) } } From 8f470b354805b2f0d5e6bd11e1ddfa27c3d89aca Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 1 Oct 2019 06:04:18 -0400 Subject: [PATCH 4/5] adjust `atc_accounting` test case --- src/lowering/test.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/lowering/test.rs b/src/lowering/test.rs index 9abb26adfdf..1d730ab781a 100644 --- a/src/lowering/test.rs +++ b/src/lowering/test.rs @@ -193,10 +193,9 @@ fn atc_accounting() { assert_eq!( &impl_text[..].replace(",\n", "\n"), &r#"ImplDatum { + polarity: Positive, binders: for ImplDatumBound { - trait_ref: Positive( - Vec<^0> as Iterable - ), + trait_ref: Vec<^0> as Iterable, where_clauses: [], associated_ty_values: [ AssociatedTyValue { @@ -207,8 +206,8 @@ fn atc_accounting() { } } ], - impl_type: Local - } + }, + impl_type: Local }"# .replace(",\n", "\n") ); From 6e1d8486d2f28ffbc7b3e1bbd4a8ec189a14bb3a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 4 Oct 2019 14:49:37 -0400 Subject: [PATCH 5/5] tmandry nits --- src/lowering.rs | 42 +++++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/lowering.rs b/src/lowering.rs index 3f11ce36878..6e84fe7b88d 100644 --- a/src/lowering.rs +++ b/src/lowering.rs @@ -780,6 +780,35 @@ impl LowerPolarity for Polarity { } } +trait LowerImplType { + fn lower(&self) -> rust_ir::ImplType; +} + +impl LowerImplType for ImplType { + fn lower(&self) -> rust_ir::ImplType { + match self { + ImplType::Local => rust_ir::ImplType::Local, + ImplType::External => rust_ir::ImplType::External, + } + } +} + +trait LowerTraitFlags { + fn lower(&self) -> rust_ir::TraitFlags; +} + +impl LowerTraitFlags for TraitFlags { + fn lower(&self) -> rust_ir::TraitFlags { + rust_ir::TraitFlags { + auto: self.auto, + marker: self.marker, + upstream: self.upstream, + fundamental: self.fundamental, + non_enumerable: self.non_enumerable, + } + } +} + trait LowerProjectionTy { fn lower(&self, env: &Env) -> Fallible; } @@ -966,10 +995,7 @@ impl LowerImpl for Impl { Ok(rust_ir::ImplDatum { polarity, binders: binders, - impl_type: match self.impl_type { - ImplType::Local => rust_ir::ImplType::Local, - ImplType::External => rust_ir::ImplType::External, - }, + impl_type: self.impl_type.lower(), }) } } @@ -1086,13 +1112,7 @@ impl LowerTrait for TraitDefn { Ok(rust_ir::TraitDatum { binders: binders, - flags: rust_ir::TraitFlags { - auto: self.flags.auto, - marker: self.flags.marker, - upstream: self.flags.upstream, - fundamental: self.flags.fundamental, - non_enumerable: self.flags.non_enumerable, - }, + flags: self.flags.lower(), }) } }