Skip to content

Commit 49dc2f9

Browse files
borsbors
committed
Auto merge of rust-lang#70272 - eddyb:type-of-impl-trait, r=nikomatsakis
typeck/type_of: let wfcheck handle generics in opaque types' substs. I was working on rust-lang#70164, and `type_of`'s handling of opaque types seemed to be, by far, the trickiest use of `Ty::walk`, but I believe it wasn't doing anything (see rust-lang#57896 (comment) - I suspect, based on glancing at the PR discussion, that an early attempt was kept in, despite becoming just an overcomplicated way to do exactly the same as the previous simple type equality check). I would've loved to remove `ResolvedOpaqueTy` (keep the `Ty` and lose the `Substs`), but it looks like the MIR borrowck part of the process needs that now, so it would've been added anyway since rust-lang#57896, even if that PR hadn't happened. <hr/> In the process, I've moved the remaining substitution validation to `wfcheck`, which was already handling lifetimes, and kept only `delay_span_bug`s in `type_of`, as an insurance policy. I've added tests for lifetime and const cases, they seem to be checked correctly now. (and more uniform than they were in rust-lang#63063 (comment)) However, the quality of the errors is maybe a bit worse, and they don't trigger when there are other errors (not sure if this is due to compilation stop points or something more specific to one opaque type). r? @nikomatsakis cc @matthewjasper @oli-obk @Aaron1011
2 parents 1b521f5 + 8ad149a commit 49dc2f9

21 files changed

+276
-311
lines changed

‎src/librustc_middle/ty/mod.rs‎

Lines changed: 21 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1077,48 +1077,42 @@ impl<'tcx> Generics {
10771077
false
10781078
}
10791079

1080+
pub fn param_at(&'tcx self, param_index: usize, tcx: TyCtxt<'tcx>) -> &'tcx GenericParamDef {
1081+
if let Some(index) = param_index.checked_sub(self.parent_count) {
1082+
&self.params[index]
1083+
} else {
1084+
tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?"))
1085+
.param_at(param_index, tcx)
1086+
}
1087+
}
1088+
10801089
pub fn region_param(
10811090
&'tcx self,
10821091
param: &EarlyBoundRegion,
10831092
tcx: TyCtxt<'tcx>,
10841093
) -> &'tcx GenericParamDef {
1085-
if let Some(index) = param.index.checked_sub(self.parent_count as u32) {
1086-
let param = &self.params[index as usize];
1087-
match param.kind {
1088-
GenericParamDefKind::Lifetime => param,
1089-
_ => bug!("expected lifetime parameter, but found another generic parameter"),
1090-
}
1091-
} else {
1092-
tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?"))
1093-
.region_param(param, tcx)
1094+
let param = self.param_at(param.index as usize, tcx);
1095+
match param.kind {
1096+
GenericParamDefKind::Lifetime => param,
1097+
_ => bug!("expected lifetime parameter, but found another generic parameter"),
10941098
}
10951099
}
10961100

10971101
/// Returns the `GenericParamDef` associated with this `ParamTy`.
10981102
pub fn type_param(&'tcx self, param: &ParamTy, tcx: TyCtxt<'tcx>) -> &'tcx GenericParamDef {
1099-
if let Some(index) = param.index.checked_sub(self.parent_count as u32) {
1100-
let param = &self.params[index as usize];
1101-
match param.kind {
1102-
GenericParamDefKind::Type { .. } => param,
1103-
_ => bug!("expected type parameter, but found another generic parameter"),
1104-
}
1105-
} else {
1106-
tcx.generics_of(self.parent.expect("parent_count > 0 but no parent?"))
1107-
.type_param(param, tcx)
1103+
let param = self.param_at(param.index as usize, tcx);
1104+
match param.kind {
1105+
GenericParamDefKind::Type { .. } => param,
1106+
_ => bug!("expected type parameter, but found another generic parameter"),
11081107
}
11091108
}
11101109

11111110
/// Returns the `ConstParameterDef` associated with this `ParamConst`.
11121111
pub fn const_param(&'tcx self, param: &ParamConst, tcx: TyCtxt<'tcx>) -> &GenericParamDef {
1113-
if let Some(index) = param.index.checked_sub(self.parent_count as u32) {
1114-
let param = &self.params[index as usize];
1115-
match param.kind {
1116-
GenericParamDefKind::Const => param,
1117-
_ => bug!("expected const parameter, but found another generic parameter"),
1118-
}
1119-
} else {
1120-
tcx.generics_of(self.parent.expect("parent_count>0 but no parent?"))
1121-
.const_param(param, tcx)
1112+
let param = self.param_at(param.index as usize, tcx);
1113+
match param.kind {
1114+
GenericParamDefKind::Const => param,
1115+
_ => bug!("expected const parameter, but found another generic parameter"),
11221116
}
11231117
}
11241118
}

‎src/librustc_typeck/check/wfcheck.rs‎

Lines changed: 49 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use rustc_hir::def_id::DefId;
99
use rustc_hir::itemlikevisit::ParItemLikeVisitor;
1010
use rustc_hir::lang_items;
1111
use rustc_hir::ItemKind;
12-
use rustc_middle::ty::subst::{InternalSubsts, Subst};
12+
use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts, Subst};
1313
use rustc_middle::ty::trait_def::TraitSpecializationKind;
1414
use rustc_middle::ty::{
1515
self, AdtKind, GenericParamDefKind, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
@@ -864,87 +864,77 @@ fn check_opaque_types<'fcx, 'tcx>(
864864
trace!("check_opaque_types: opaque_ty, {:?}, {:?}", def_id, substs);
865865
let generics = tcx.generics_of(def_id);
866866
// Only check named `impl Trait` types defined in this crate.
867+
// FIXME(eddyb) is `generics.parent.is_none()` correct? It seems
868+
// potentially risky wrt associated types in `impl`s.
867869
if generics.parent.is_none() && def_id.is_local() {
868870
let opaque_hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
869871
if may_define_opaque_type(tcx, fn_def_id, opaque_hir_id) {
870872
trace!("check_opaque_types: may define, generics={:#?}", generics);
871-
let mut seen: FxHashMap<_, Vec<_>> = FxHashMap::default();
872-
for (subst, param) in substs.iter().zip(&generics.params) {
873-
match subst.unpack() {
874-
ty::subst::GenericArgKind::Type(ty) => match ty.kind {
875-
ty::Param(..) => {}
876-
// Prevent `fn foo() -> Foo<u32>` from being defining.
877-
_ => {
878-
tcx.sess
879-
.struct_span_err(
880-
span,
881-
"non-defining opaque type use \
882-
in defining scope",
883-
)
884-
.span_note(
885-
tcx.def_span(param.def_id),
886-
&format!(
887-
"used non-generic type {} for \
888-
generic parameter",
889-
ty,
890-
),
891-
)
892-
.emit();
893-
}
894-
},
873+
let mut seen_params: FxHashMap<_, Vec<_>> = FxHashMap::default();
874+
for (i, &arg) in substs.iter().enumerate() {
875+
let arg_is_param = match arg.unpack() {
876+
GenericArgKind::Type(ty) => matches!(ty.kind, ty::Param(_)),
895877

896-
ty::subst::GenericArgKind::Lifetime(region) => {
897-
let param_span = tcx.def_span(param.def_id);
878+
GenericArgKind::Lifetime(region) => {
898879
if let ty::ReStatic = region {
899880
tcx.sess
900881
.struct_span_err(
901882
span,
902-
"non-defining opaque type use \
903-
in defining scope",
883+
"non-defining opaque type use in defining scope",
904884
)
905885
.span_label(
906-
param_span,
886+
tcx.def_span(generics.param_at(i, tcx).def_id),
907887
"cannot use static lifetime; use a bound lifetime \
908888
instead or remove the lifetime parameter from the \
909889
opaque type",
910890
)
911891
.emit();
912-
} else {
913-
seen.entry(region).or_default().push(param_span);
892+
continue;
914893
}
894+
895+
true
915896
}
916897

917-
ty::subst::GenericArgKind::Const(ct) => match ct.val {
918-
ty::ConstKind::Param(_) => {}
919-
_ => {
920-
tcx.sess
921-
.struct_span_err(
922-
span,
923-
"non-defining opaque type use \
924-
in defining scope",
925-
)
926-
.span_note(
927-
tcx.def_span(param.def_id),
928-
&format!(
929-
"used non-generic const {} for \
930-
generic parameter",
931-
ty,
932-
),
933-
)
934-
.emit();
935-
}
936-
},
937-
} // match subst
938-
} // for (subst, param)
939-
for (_, spans) in seen {
940-
if spans.len() > 1 {
898+
GenericArgKind::Const(ct) => {
899+
matches!(ct.val, ty::ConstKind::Param(_))
900+
}
901+
};
902+
903+
if arg_is_param {
904+
seen_params.entry(arg).or_default().push(i);
905+
} else {
906+
// Prevent `fn foo() -> Foo<u32>` from being defining.
907+
let opaque_param = generics.param_at(i, tcx);
908+
tcx.sess
909+
.struct_span_err(
910+
span,
911+
"non-defining opaque type use in defining scope",
912+
)
913+
.span_note(
914+
tcx.def_span(opaque_param.def_id),
915+
&format!(
916+
"used non-generic {} `{}` for generic parameter",
917+
opaque_param.kind.descr(),
918+
arg,
919+
),
920+
)
921+
.emit();
922+
}
923+
} // for (arg, param)
924+
925+
for (_, indices) in seen_params {
926+
if indices.len() > 1 {
927+
let descr = generics.param_at(indices[0], tcx).kind.descr();
928+
let spans: Vec<_> = indices
929+
.into_iter()
930+
.map(|i| tcx.def_span(generics.param_at(i, tcx).def_id))
931+
.collect();
941932
tcx.sess
942933
.struct_span_err(
943934
span,
944-
"non-defining opaque type use \
945-
in defining scope",
935+
"non-defining opaque type use in defining scope",
946936
)
947-
.span_note(spans, "lifetime used multiple times")
937+
.span_note(spans, &format!("{} used multiple times", descr))
948938
.emit();
949939
}
950940
}

0 commit comments

Comments
 (0)