From 64651138524b45a7e18bec7dcaf143c7fa4f2794 Mon Sep 17 00:00:00 2001 From: Eduard-Mihai Burtescu Date: Sun, 22 Mar 2020 11:01:46 +0200 Subject: [PATCH] typeck/type_of: simplify checking of opaque types with multipler defining uses. --- src/librustc_middle/ty/mod.rs | 48 +++---- src/librustc_typeck/collect/type_of.rs | 121 +++++------------- .../generic_duplicate_param_use3.rs | 2 +- .../generic_duplicate_param_use3.stderr | 4 +- 4 files changed, 53 insertions(+), 122 deletions(-) diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 901365ef691..9b182333907 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -1077,48 +1077,42 @@ pub fn own_requires_monomorphization(&self) -> bool { false } + pub fn param_at(&'tcx self, param_index: usize, tcx: TyCtxt<'tcx>) -> &'tcx GenericParamDef { + if let Some(index) = param_index.checked_sub(self.parent_count) { + &self.params[index] + } else { + tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?")) + .param_at(param_index, tcx) + } + } + pub fn region_param( &'tcx self, param: &EarlyBoundRegion, tcx: TyCtxt<'tcx>, ) -> &'tcx GenericParamDef { - if let Some(index) = param.index.checked_sub(self.parent_count as u32) { - let param = &self.params[index as usize]; - match param.kind { - GenericParamDefKind::Lifetime => param, - _ => bug!("expected lifetime parameter, but found another generic parameter"), - } - } else { - tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?")) - .region_param(param, tcx) + let param = self.param_at(param.index as usize, tcx); + match param.kind { + GenericParamDefKind::Lifetime => param, + _ => bug!("expected lifetime parameter, but found another generic parameter"), } } /// Returns the `GenericParamDef` associated with this `ParamTy`. pub fn type_param(&'tcx self, param: &ParamTy, tcx: TyCtxt<'tcx>) -> &'tcx GenericParamDef { - if let Some(index) = param.index.checked_sub(self.parent_count as u32) { - let param = &self.params[index as usize]; - match param.kind { - GenericParamDefKind::Type { .. } => param, - _ => bug!("expected type parameter, but found another generic parameter"), - } - } else { - tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?")) - .type_param(param, tcx) + let param = self.param_at(param.index as usize, tcx); + match param.kind { + GenericParamDefKind::Type { .. } => param, + _ => bug!("expected type parameter, but found another generic parameter"), } } /// Returns the `ConstParameterDef` associated with this `ParamConst`. pub fn const_param(&'tcx self, param: &ParamConst, tcx: TyCtxt<'tcx>) -> &GenericParamDef { - if let Some(index) = param.index.checked_sub(self.parent_count as u32) { - let param = &self.params[index as usize]; - match param.kind { - GenericParamDefKind::Const => param, - _ => bug!("expected const parameter, but found another generic parameter"), - } - } else { - tcx.generics_of(self.parent.expect("parent_count>0 but no parent?")) - .const_param(param, tcx) + let param = self.param_at(param.index as usize, tcx); + match param.kind { + GenericParamDefKind::Const => param, + _ => bug!("expected const parameter, but found another generic parameter"), } } } diff --git a/src/librustc_typeck/collect/type_of.rs b/src/librustc_typeck/collect/type_of.rs index e6099b98dc8..acf593c2912 100644 --- a/src/librustc_typeck/collect/type_of.rs +++ b/src/librustc_typeck/collect/type_of.rs @@ -1,4 +1,4 @@ -use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::fx::FxHashSet; use rustc_errors::{struct_span_err, Applicability, StashKey}; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; @@ -7,7 +7,7 @@ use rustc_hir::intravisit::Visitor; use rustc_hir::Node; use rustc_middle::hir::map::Map; -use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts, Subst}; +use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts}; use rustc_middle::ty::util::IntTypeExt; use rustc_middle::ty::{self, DefIdTree, Ty, TyCtxt, TypeFoldable}; use rustc_session::parse::feature_err; @@ -369,13 +369,8 @@ fn find_opaque_ty_constraints(tcx: TyCtxt<'_>, def_id: DefId) -> Ty<'_> { struct ConstraintLocator<'tcx> { tcx: TyCtxt<'tcx>, def_id: DefId, - // (first found type span, actual type, mapping from the opaque type's generic - // parameters to the concrete type's generic parameters) - // - // The mapping is an index for each use site of a generic parameter in the concrete type - // - // The indices index into the generic parameters on the opaque type. - found: Option<(Span, Ty<'tcx>, Vec)>, + // (first found type span, actual type) + found: Option<(Span, Ty<'tcx>)>, } impl ConstraintLocator<'_> { @@ -407,14 +402,15 @@ fn check(&mut self, def_id: DefId) { // FIXME(oli-obk): trace the actual span from inference to improve errors. let span = self.tcx.def_span(def_id); - // used to quickly look up the position of a generic parameter - let mut index_map: FxHashMap = FxHashMap::default(); - // Skipping binder is ok, since we only use this to find generic parameters and - // their positions. - for (idx, subst) in substs.iter().enumerate() { - if let GenericArgKind::Type(ty) = subst.unpack() { + + let opaque_generics = self.tcx.generics_of(self.def_id); + let mut used_params: FxHashSet = FxHashSet::default(); + let mut has_errors = false; + for (i, arg) in substs.iter().enumerate() { + // FIXME(eddyb) enforce lifetime and const param 1:1 mapping. + if let GenericArgKind::Type(ty) = arg.unpack() { if let ty::Param(p) = ty.kind { - if index_map.insert(p, idx).is_some() { + if !used_params.insert(p) { // There was already an entry for `p`, meaning a generic parameter // was used twice. self.tcx.sess.span_err( @@ -428,62 +424,28 @@ fn check(&mut self, def_id: DefId) { return; } } else { - self.tcx.sess.delay_span_bug( + let param = opaque_generics.param_at(i, self.tcx); + self.tcx.sess.span_err( span, &format!( - "non-defining opaque ty use in defining scope: {:?}, {:?}", - concrete_type, substs, + "defining opaque type use does not fully define opaque type: \ + generic parameter `{}` is specified as concrete {} `{}`", + param.name, + param.kind.descr(), + arg, ), ); + has_errors = true; } } } - // Compute the index within the opaque type for each generic parameter used in - // the concrete type. - let indices = concrete_type - .subst(self.tcx, substs) - .walk() - .filter_map(|t| match &t.kind { - ty::Param(p) => Some(*index_map.get(p).unwrap()), - _ => None, - }) - .collect(); - let is_param = |ty: Ty<'_>| match ty.kind { - ty::Param(_) => true, - _ => false, - }; - let bad_substs: Vec<_> = substs - .iter() - .enumerate() - .filter_map(|(i, k)| { - if let GenericArgKind::Type(ty) = k.unpack() { Some((i, ty)) } else { None } - }) - .filter(|(_, ty)| !is_param(ty)) - .collect(); - if !bad_substs.is_empty() { - let identity_substs = InternalSubsts::identity_for_item(self.tcx, self.def_id); - for (i, bad_subst) in bad_substs { - self.tcx.sess.span_err( - span, - &format!( - "defining opaque type use does not fully define opaque type: \ - generic parameter `{}` is specified as concrete type `{}`", - identity_substs.type_at(i), - bad_subst - ), - ); - } - } else if let Some((prev_span, prev_ty, ref prev_indices)) = self.found { - let mut ty = concrete_type.walk().fuse(); - let mut p_ty = prev_ty.walk().fuse(); - let iter_eq = (&mut ty).zip(&mut p_ty).all(|(t, p)| match (&t.kind, &p.kind) { - // Type parameters are equal to any other type parameter for the purpose of - // concrete type equality, as it is possible to obtain the same type just - // by passing matching parameters to a function. - (ty::Param(_), ty::Param(_)) => true, - _ => t == p, - }); - if !iter_eq || ty.next().is_some() || p_ty.next().is_some() { + + if has_errors { + return; + } + + if let Some((prev_span, prev_ty)) = self.found { + if *concrete_type != prev_ty { debug!("find_opaque_ty_constraints: span={:?}", span); // Found different concrete types for the opaque type. let mut err = self.tcx.sess.struct_span_err( @@ -496,34 +458,9 @@ fn check(&mut self, def_id: DefId) { ); err.span_note(prev_span, "previous use here"); err.emit(); - } else if indices != *prev_indices { - // Found "same" concrete types, but the generic parameter order differs. - let mut err = self.tcx.sess.struct_span_err( - span, - "concrete type's generic parameters differ from previous defining use", - ); - use std::fmt::Write; - let mut s = String::new(); - write!(s, "expected [").unwrap(); - let list = |s: &mut String, indices: &Vec| { - let mut indices = indices.iter().cloned(); - if let Some(first) = indices.next() { - write!(s, "`{}`", substs[first]).unwrap(); - for i in indices { - write!(s, ", `{}`", substs[i]).unwrap(); - } - } - }; - list(&mut s, prev_indices); - write!(s, "], got [").unwrap(); - list(&mut s, &indices); - write!(s, "]").unwrap(); - err.span_label(span, s); - err.span_note(prev_span, "previous use here"); - err.emit(); } } else { - self.found = Some((span, concrete_type, indices)); + self.found = Some((span, concrete_type)); } } else { debug!( @@ -606,7 +543,7 @@ fn visit_trait_item(&mut self, it: &'tcx TraitItem<'tcx>) { } match locator.found { - Some((_, ty, _)) => ty, + Some((_, ty)) => ty, None => { let span = tcx.def_span(def_id); tcx.sess.span_err(span, "could not find defining uses"); diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.rs b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.rs index 8d3e7f9f424..ec9f40851c5 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.rs +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.rs @@ -17,6 +17,6 @@ fn two(t: T, _: U) -> Two { } fn three(_: T, u: U) -> Two { -//~^ concrete type's generic parameters differ from previous defining use +//~^ concrete type differs from previous defining opaque type use u } diff --git a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.stderr b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.stderr index 04dcdc295f9..51bee41aba3 100644 --- a/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.stderr +++ b/src/test/ui/type-alias-impl-trait/generic_duplicate_param_use3.stderr @@ -7,14 +7,14 @@ LL | | t LL | | } | |_^ -error: concrete type's generic parameters differ from previous defining use +error: concrete type differs from previous defining opaque type use --> $DIR/generic_duplicate_param_use3.rs:19:1 | LL | / fn three(_: T, u: U) -> Two { LL | | LL | | u LL | | } - | |_^ expected [`T`], got [`U`] + | |_^ expected `T`, got `U` | note: previous use here --> $DIR/generic_duplicate_param_use3.rs:15:1