Skip to content

Commit

Permalink
Auto merge of #70272 - eddyb:type-of-impl-trait, r=nikomatsakis
Browse files Browse the repository at this point in the history 
typeck/type_of: let wfcheck handle generics in opaque types' substs.

I was working on #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 #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 #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 #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
  • Loading branch information
@bors
bors committed Apr 4, 2020
2 parents 1b521f5 + 8ad149a commit 49dc2f9
Show file tree
Hide file tree
Showing 21 changed files with 276 additions and 311 deletions.
48 changes: 21 additions & 27 deletions src/librustc_middle/ty/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1077,48 +1077,42 @@ impl<'tcx> Generics {
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"),
}
}
}
Expand Down
108 changes: 49 additions & 59 deletions src/librustc_typeck/check/wfcheck.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use rustc_hir::def_id::DefId;
use rustc_hir::itemlikevisit::ParItemLikeVisitor;
use rustc_hir::lang_items;
use rustc_hir::ItemKind;
use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts, Subst};
use rustc_middle::ty::trait_def::TraitSpecializationKind;
use rustc_middle::ty::{
self, AdtKind, GenericParamDefKind, ToPredicate, Ty, TyCtxt, TypeFoldable, WithConstness,
Expand Down Expand Up @@ -864,87 +864,77 @@ fn check_opaque_types<'fcx, 'tcx>(
trace!("check_opaque_types: opaque_ty, {:?}, {:?}", def_id, substs);
let generics = tcx.generics_of(def_id);
// Only check named `impl Trait` types defined in this crate.
// FIXME(eddyb) is `generics.parent.is_none()` correct? It seems
// potentially risky wrt associated types in `impl`s.
if generics.parent.is_none() && def_id.is_local() {
let opaque_hir_id = tcx.hir().as_local_hir_id(def_id).unwrap();
if may_define_opaque_type(tcx, fn_def_id, opaque_hir_id) {
trace!("check_opaque_types: may define, generics={:#?}", generics);
let mut seen: FxHashMap<_, Vec<_>> = FxHashMap::default();
for (subst, param) in substs.iter().zip(&generics.params) {
match subst.unpack() {
ty::subst::GenericArgKind::Type(ty) => match ty.kind {
ty::Param(..) => {}
// Prevent `fn foo() -> Foo<u32>` from being defining.
_ => {
tcx.sess
.struct_span_err(
span,
"non-defining opaque type use \
in defining scope",
)
.span_note(
tcx.def_span(param.def_id),
&format!(
"used non-generic type {} for \
generic parameter",
ty,
),
)
.emit();
}
},
let mut seen_params: FxHashMap<_, Vec<_>> = FxHashMap::default();
for (i, &arg) in substs.iter().enumerate() {
let arg_is_param = match arg.unpack() {
GenericArgKind::Type(ty) => matches!(ty.kind, ty::Param(_)),

ty::subst::GenericArgKind::Lifetime(region) => {
let param_span = tcx.def_span(param.def_id);
GenericArgKind::Lifetime(region) => {
if let ty::ReStatic = region {
tcx.sess
.struct_span_err(
span,
"non-defining opaque type use \
in defining scope",
"non-defining opaque type use in defining scope",
)
.span_label(
param_span,
tcx.def_span(generics.param_at(i, tcx).def_id),
"cannot use static lifetime; use a bound lifetime \
instead or remove the lifetime parameter from the \
opaque type",
)
.emit();
} else {
seen.entry(region).or_default().push(param_span);
continue;
}

true
}

ty::subst::GenericArgKind::Const(ct) => match ct.val {
ty::ConstKind::Param(_) => {}
_ => {
tcx.sess
.struct_span_err(
span,
"non-defining opaque type use \
in defining scope",
)
.span_note(
tcx.def_span(param.def_id),
&format!(
"used non-generic const {} for \
generic parameter",
ty,
),
)
.emit();
}
},
} // match subst
} // for (subst, param)
for (_, spans) in seen {
if spans.len() > 1 {
GenericArgKind::Const(ct) => {
matches!(ct.val, ty::ConstKind::Param(_))
}
};

if arg_is_param {
seen_params.entry(arg).or_default().push(i);
} else {
// Prevent `fn foo() -> Foo<u32>` from being defining.
let opaque_param = generics.param_at(i, tcx);
tcx.sess
.struct_span_err(
span,
"non-defining opaque type use in defining scope",
)
.span_note(
tcx.def_span(opaque_param.def_id),
&format!(
"used non-generic {} `{}` for generic parameter",
opaque_param.kind.descr(),
arg,
),
)
.emit();
}
} // for (arg, param)

for (_, indices) in seen_params {
if indices.len() > 1 {
let descr = generics.param_at(indices[0], tcx).kind.descr();
let spans: Vec<_> = indices
.into_iter()
.map(|i| tcx.def_span(generics.param_at(i, tcx).def_id))
.collect();
tcx.sess
.struct_span_err(
span,
"non-defining opaque type use \
in defining scope",
"non-defining opaque type use in defining scope",
)
.span_note(spans, "lifetime used multiple times")
.span_note(spans, &format!("{} used multiple times", descr))
.emit();
}
}
Expand Down
Loading

0 comments on commit 49dc2f9

Please sign in to comment.