From 9f7d0e91b528a19a9516df0f647a0191808e7227 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Mon, 11 Dec 2023 13:03:35 +0000 Subject: [PATCH 1/2] use Vec for region constraints --- .../src/infer/lexical_region_resolve/mod.rs | 12 ++++++++--- .../infer/region_constraints/leak_check.rs | 8 +++---- .../src/infer/region_constraints/mod.rs | 21 +++++++------------ .../src/traits/auto_trait.rs | 2 +- src/librustdoc/clean/auto_trait.rs | 2 +- .../higher-ranked-lifetime-error.stderr | 4 ++-- .../trait-bounds/hrtb-perfect-forwarding.rs | 2 +- .../hrtb-perfect-forwarding.stderr | 14 +++++++++---- tests/ui/lifetimes/issue-79187-2.stderr | 18 ++++++++++++---- 9 files changed, 50 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs index b6e86e2b676d0..08e6d35a5f59f 100644 --- a/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs +++ b/compiler/rustc_infer/src/infer/lexical_region_resolve/mod.rs @@ -137,6 +137,10 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { ) -> LexicalRegionResolutions<'tcx> { let mut var_data = self.construct_var_data(); + // Deduplicating constraints is shown to have a positive perf impact. + self.data.constraints.sort_by_key(|(constraint, _)| *constraint); + self.data.constraints.dedup_by_key(|(constraint, _)| *constraint); + if cfg!(debug_assertions) { self.dump_constraints(); } @@ -183,7 +187,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { let mut constraints = IndexVec::from_elem(Vec::new(), &var_values.values); // Tracks the changed region vids. let mut changes = Vec::new(); - for constraint in self.data.constraints.keys() { + for (constraint, _) in &self.data.constraints { match *constraint { Constraint::RegSubVar(a_region, b_vid) => { let b_data = var_values.value_mut(b_vid); @@ -678,7 +682,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { let dummy_source = graph.add_node(()); let dummy_sink = graph.add_node(()); - for constraint in self.data.constraints.keys() { + for (constraint, _) in &self.data.constraints { match *constraint { Constraint::VarSubVar(a_id, b_id) => { graph.add_edge(NodeIndex(a_id.index()), NodeIndex(b_id.index()), *constraint); @@ -885,9 +889,11 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> { } Constraint::RegSubVar(region, _) | Constraint::VarSubReg(_, region) => { + let constraint_idx = + this.constraints.binary_search_by(|(c, _)| c.cmp(&edge.data)).unwrap(); state.result.push(RegionAndOrigin { region, - origin: this.constraints.get(&edge.data).unwrap().clone(), + origin: this.constraints[constraint_idx].1.clone(), }); } diff --git a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs index e06596df7b94a..b2bcbbf2e53be 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/leak_check.rs @@ -416,8 +416,8 @@ impl<'tcx> MiniGraph<'tcx> { region_constraints.undo_log.region_constraints_in_snapshot(&snapshot.undo_snapshot) { match undo_entry { - AddConstraint(constraint) => { - each_constraint(constraint); + &AddConstraint(i) => { + each_constraint(®ion_constraints.data().constraints[i].0); } &AddVerify(i) => span_bug!( region_constraints.data().verifys[i].origin.span(), @@ -430,8 +430,8 @@ impl<'tcx> MiniGraph<'tcx> { region_constraints .data() .constraints - .keys() - .for_each(|constraint| each_constraint(constraint)); + .iter() + .for_each(|(constraint, _)| each_constraint(constraint)); } } diff --git a/compiler/rustc_infer/src/infer/region_constraints/mod.rs b/compiler/rustc_infer/src/infer/region_constraints/mod.rs index 5c043b1d3dd64..c06adf085aa2c 100644 --- a/compiler/rustc_infer/src/infer/region_constraints/mod.rs +++ b/compiler/rustc_infer/src/infer/region_constraints/mod.rs @@ -20,7 +20,6 @@ use rustc_middle::ty::{ReBound, ReVar}; use rustc_middle::ty::{Region, RegionVid}; use rustc_span::Span; -use std::collections::BTreeMap; use std::ops::Range; use std::{cmp, fmt, mem}; @@ -90,7 +89,7 @@ pub type VarInfos = IndexVec; pub struct RegionConstraintData<'tcx> { /// Constraints of the form `A <= B`, where either `A` or `B` can /// be a region variable (or neither, as it happens). - pub constraints: BTreeMap, SubregionOrigin<'tcx>>, + pub constraints: Vec<(Constraint<'tcx>, SubregionOrigin<'tcx>)>, /// Constraints of the form `R0 member of [R1, ..., Rn]`, meaning that /// `R0` must be equal to one of the regions `R1..Rn`. These occur @@ -273,7 +272,7 @@ pub(crate) enum UndoLog<'tcx> { AddVar(RegionVid), /// We added the given `constraint`. - AddConstraint(Constraint<'tcx>), + AddConstraint(usize), /// We added the given `verify`. AddVerify(usize), @@ -319,8 +318,9 @@ impl<'tcx> RegionConstraintStorage<'tcx> { self.var_infos.pop().unwrap(); assert_eq!(self.var_infos.len(), vid.index()); } - AddConstraint(ref constraint) => { - self.data.constraints.remove(constraint); + AddConstraint(index) => { + self.data.constraints.pop().unwrap(); + assert_eq!(self.data.constraints.len(), index); } AddVerify(index) => { self.data.verifys.pop(); @@ -443,14 +443,9 @@ impl<'tcx> RegionConstraintCollector<'_, 'tcx> { // cannot add constraints once regions are resolved debug!("RegionConstraintCollector: add_constraint({:?})", constraint); - // never overwrite an existing (constraint, origin) - only insert one if it isn't - // present in the map yet. This prevents origins from outside the snapshot being - // replaced with "less informative" origins e.g., during calls to `can_eq` - let undo_log = &mut self.undo_log; - self.storage.data.constraints.entry(constraint).or_insert_with(|| { - undo_log.push(AddConstraint(constraint)); - origin - }); + let index = self.storage.data.constraints.len(); + self.storage.data.constraints.push((constraint, origin)); + self.undo_log.push(AddConstraint(index)); } fn add_verify(&mut self, verify: Verify<'tcx>) { diff --git a/compiler/rustc_trait_selection/src/traits/auto_trait.rs b/compiler/rustc_trait_selection/src/traits/auto_trait.rs index 13a09917c033e..c6d029ddb650a 100644 --- a/compiler/rustc_trait_selection/src/traits/auto_trait.rs +++ b/compiler/rustc_trait_selection/src/traits/auto_trait.rs @@ -477,7 +477,7 @@ impl<'tcx> AutoTraitFinder<'tcx> { let mut vid_map: FxHashMap, RegionDeps<'cx>> = FxHashMap::default(); let mut finished_map = FxHashMap::default(); - for constraint in regions.constraints.keys() { + for (constraint, _) in ®ions.constraints { match constraint { &Constraint::VarSubVar(r1, r2) => { { diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs index e692f4ef72ea4..9de547ba6dcef 100644 --- a/src/librustdoc/clean/auto_trait.rs +++ b/src/librustdoc/clean/auto_trait.rs @@ -195,7 +195,7 @@ where // into a map. Each RegionTarget (either a RegionVid or a Region) maps // to its smaller and larger regions. Note that 'larger' regions correspond // to sub-regions in Rust code (e.g., in 'a: 'b, 'a is the larger region). - for constraint in regions.constraints.keys() { + for (constraint, _) in ®ions.constraints { match *constraint { Constraint::VarSubVar(r1, r2) => { { diff --git a/tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr b/tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr index d0892fd8b0953..c25e731d9627c 100644 --- a/tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr +++ b/tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr @@ -4,8 +4,8 @@ error[E0308]: mismatched types LL | assert_all::<_, &String>(id); | ^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected reference `&String` - found reference `&String` + = note: expected trait `for<'a> fn(&'a String) -> &'a String {id} as FnMut<(&'a String,)>>` + found trait `for<'a> fn(&'a String) -> &'a String {id} as FnMut<(&'a String,)>>` error: aborting due to 1 previous error diff --git a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs index d45fa183c0c4c..07befeff43cb9 100644 --- a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs +++ b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs @@ -41,7 +41,7 @@ where // isize>`, we require `T : for<'a> Bar<&'a isize>`, but the where // clause only specifies `T : Bar<&'b isize>`. foo_hrtb_bar_not(&mut t); - //~^ ERROR implementation of `Bar` is not general enough + //~^ ERROR mismatched types //~^^ ERROR lifetime may not live long enough } diff --git a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr index 727b9e6bec8e6..14a80630f408a 100644 --- a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr +++ b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr @@ -53,14 +53,19 @@ note: due to current limitations in the borrow checker, this implies a `'static` LL | T: for<'a> Foo<&'a isize> + Bar<&'b isize>, | ^^^^^^^^^^^^^^^^^^^^^^ -error: implementation of `Bar` is not general enough +error[E0308]: mismatched types --> $DIR/hrtb-perfect-forwarding.rs:43:5 | LL | foo_hrtb_bar_not(&mut t); - | ^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough + | ^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... - = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` + = note: expected trait `for<'a> >` + found trait `for<'a> >` +note: the lifetime requirement is introduced here + --> $DIR/hrtb-perfect-forwarding.rs:37:8 + | +LL | T: for<'a> Foo<&'a isize> + Bar<&'b isize>, + | ^^^^^^^^^^^^^^^^^^^^^^ warning: function cannot return without recursing --> $DIR/hrtb-perfect-forwarding.rs:48:1 @@ -77,3 +82,4 @@ LL | foo_hrtb_bar_hrtb(&mut t); error: aborting due to 2 previous errors; 4 warnings emitted +For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/lifetimes/issue-79187-2.stderr b/tests/ui/lifetimes/issue-79187-2.stderr index e8115bb6b064f..0b7593f01c611 100644 --- a/tests/ui/lifetimes/issue-79187-2.stderr +++ b/tests/ui/lifetimes/issue-79187-2.stderr @@ -54,8 +54,13 @@ error[E0308]: mismatched types LL | take_foo(|a: &i32| a); | ^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected reference `&_` - found reference `&_` + = note: expected trait `for<'a> <{closure@$DIR/issue-79187-2.rs:11:14: 11:23} as FnOnce<(&'a i32,)>>` + found trait `for<'a> <{closure@$DIR/issue-79187-2.rs:11:14: 11:23} as FnOnce<(&'a i32,)>>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-79187-2.rs:11:14 + | +LL | take_foo(|a: &i32| a); + | ^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-79187-2.rs:5:21 | @@ -68,8 +73,13 @@ error[E0308]: mismatched types LL | take_foo(|a: &i32| -> &i32 { a }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected reference `&_` - found reference `&_` + = note: expected trait `for<'a> <{closure@$DIR/issue-79187-2.rs:14:14: 14:31} as FnOnce<(&'a i32,)>>` + found trait `for<'a> <{closure@$DIR/issue-79187-2.rs:14:14: 14:31} as FnOnce<(&'a i32,)>>` +note: this closure does not fulfill the lifetime requirements + --> $DIR/issue-79187-2.rs:14:14 + | +LL | take_foo(|a: &i32| -> &i32 { a }); + | ^^^^^^^^^^^^^^^^^ note: the lifetime requirement is introduced here --> $DIR/issue-79187-2.rs:5:21 | From 8c215e7841c486f5f460d541bbc1773933266475 Mon Sep 17 00:00:00 2001 From: Ali MJ Al-Nasrawy Date: Sun, 17 Dec 2023 06:08:24 +0000 Subject: [PATCH 2/2] fix diagnostic regresssion --- .../src/diagnostics/bound_region_errors.rs | 19 ++++++++++++------- .../trait-bounds/hrtb-perfect-forwarding.rs | 2 +- .../hrtb-perfect-forwarding.stderr | 14 ++++---------- tests/ui/lifetimes/issue-79187-2.stderr | 18 ++++-------------- 4 files changed, 21 insertions(+), 32 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs index 924e68fa91dac..60dbf7dc055bf 100644 --- a/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs @@ -408,13 +408,18 @@ fn try_extract_error_from_region_constraints<'tcx>( mut region_var_origin: impl FnMut(RegionVid) -> RegionVariableOrigin, mut universe_of_region: impl FnMut(RegionVid) -> UniverseIndex, ) -> Option> { + let placeholder_universe = match placeholder_region.kind() { + ty::RePlaceholder(p) => p.universe, + ty::ReVar(vid) => universe_of_region(vid), + _ => ty::UniverseIndex::ROOT, + }; let matches = |a_region: Region<'tcx>, b_region: Region<'tcx>| match (a_region.kind(), b_region.kind()) { (RePlaceholder(a_p), RePlaceholder(b_p)) => a_p.bound == b_p.bound, _ => a_region == b_region, }; - let check = |constraint: &Constraint<'tcx>, cause: &SubregionOrigin<'tcx>, exact| { - match *constraint { + let mut check = + |constraint: &Constraint<'tcx>, cause: &SubregionOrigin<'tcx>, exact| match *constraint { Constraint::RegSubReg(sub, sup) if ((exact && sup == placeholder_region) || (!exact && matches(sup, placeholder_region))) @@ -422,16 +427,16 @@ fn try_extract_error_from_region_constraints<'tcx>( { Some((sub, cause.clone())) } - // FIXME: Should this check the universe of the var? Constraint::VarSubReg(vid, sup) - if ((exact && sup == placeholder_region) - || (!exact && matches(sup, placeholder_region))) => + if (exact + && sup == placeholder_region + && !universe_of_region(vid).can_name(placeholder_universe)) + || (!exact && matches(sup, placeholder_region)) => { Some((ty::Region::new_var(infcx.tcx, vid), cause.clone())) } _ => None, - } - }; + }; let mut info = region_constraints .constraints .iter() diff --git a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs index 07befeff43cb9..d45fa183c0c4c 100644 --- a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs +++ b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.rs @@ -41,7 +41,7 @@ where // isize>`, we require `T : for<'a> Bar<&'a isize>`, but the where // clause only specifies `T : Bar<&'b isize>`. foo_hrtb_bar_not(&mut t); - //~^ ERROR mismatched types + //~^ ERROR implementation of `Bar` is not general enough //~^^ ERROR lifetime may not live long enough } diff --git a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr index 14a80630f408a..727b9e6bec8e6 100644 --- a/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr +++ b/tests/ui/higher-ranked/trait-bounds/hrtb-perfect-forwarding.stderr @@ -53,19 +53,14 @@ note: due to current limitations in the borrow checker, this implies a `'static` LL | T: for<'a> Foo<&'a isize> + Bar<&'b isize>, | ^^^^^^^^^^^^^^^^^^^^^^ -error[E0308]: mismatched types +error: implementation of `Bar` is not general enough --> $DIR/hrtb-perfect-forwarding.rs:43:5 | LL | foo_hrtb_bar_not(&mut t); - | ^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other + | ^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `Bar` is not general enough | - = note: expected trait `for<'a> >` - found trait `for<'a> >` -note: the lifetime requirement is introduced here - --> $DIR/hrtb-perfect-forwarding.rs:37:8 - | -LL | T: for<'a> Foo<&'a isize> + Bar<&'b isize>, - | ^^^^^^^^^^^^^^^^^^^^^^ + = note: `T` must implement `Bar<&'0 isize>`, for any lifetime `'0`... + = note: ...but it actually implements `Bar<&'1 isize>`, for some specific lifetime `'1` warning: function cannot return without recursing --> $DIR/hrtb-perfect-forwarding.rs:48:1 @@ -82,4 +77,3 @@ LL | foo_hrtb_bar_hrtb(&mut t); error: aborting due to 2 previous errors; 4 warnings emitted -For more information about this error, try `rustc --explain E0308`. diff --git a/tests/ui/lifetimes/issue-79187-2.stderr b/tests/ui/lifetimes/issue-79187-2.stderr index 0b7593f01c611..e8115bb6b064f 100644 --- a/tests/ui/lifetimes/issue-79187-2.stderr +++ b/tests/ui/lifetimes/issue-79187-2.stderr @@ -54,13 +54,8 @@ error[E0308]: mismatched types LL | take_foo(|a: &i32| a); | ^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> <{closure@$DIR/issue-79187-2.rs:11:14: 11:23} as FnOnce<(&'a i32,)>>` - found trait `for<'a> <{closure@$DIR/issue-79187-2.rs:11:14: 11:23} as FnOnce<(&'a i32,)>>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-79187-2.rs:11:14 - | -LL | take_foo(|a: &i32| a); - | ^^^^^^^^^ + = note: expected reference `&_` + found reference `&_` note: the lifetime requirement is introduced here --> $DIR/issue-79187-2.rs:5:21 | @@ -73,13 +68,8 @@ error[E0308]: mismatched types LL | take_foo(|a: &i32| -> &i32 { a }); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other | - = note: expected trait `for<'a> <{closure@$DIR/issue-79187-2.rs:14:14: 14:31} as FnOnce<(&'a i32,)>>` - found trait `for<'a> <{closure@$DIR/issue-79187-2.rs:14:14: 14:31} as FnOnce<(&'a i32,)>>` -note: this closure does not fulfill the lifetime requirements - --> $DIR/issue-79187-2.rs:14:14 - | -LL | take_foo(|a: &i32| -> &i32 { a }); - | ^^^^^^^^^^^^^^^^^ + = note: expected reference `&_` + found reference `&_` note: the lifetime requirement is introduced here --> $DIR/issue-79187-2.rs:5:21 |