Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that all hidden types are the same and then deduplicate them. #95731

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Check that all hidden types are the same and then deduplicate them. #95731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 80 additions & 62 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,75 +55,93 @@ impl<'tcx> RegionInferenceContext<'tcx> {
infcx: &InferCtxt<'_, 'tcx>,
opaque_ty_decls: VecMap<OpaqueTypeKey<'tcx>, (OpaqueHiddenType<'tcx>, OpaqueTyOrigin)>,
) -> VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>> {
opaque_ty_decls
.into_iter()
.map(|(opaque_type_key, (concrete_type, origin))| {
let substs = opaque_type_key.substs;
debug!(?concrete_type, ?substs);
let mut result: VecMap<OpaqueTypeKey<'tcx>, OpaqueHiddenType<'tcx>> = VecMap::new();
for (opaque_type_key, (concrete_type, origin)) in opaque_ty_decls {
let substs = opaque_type_key.substs;
debug!(?concrete_type, ?substs);

let mut subst_regions = vec![self.universal_regions.fr_static];
let universal_substs = infcx.tcx.fold_regions(substs, &mut false, |region, _| {
if let ty::RePlaceholder(..) = region.kind() {
// Higher kinded regions don't need remapping, they don't refer to anything outside of this the substs.
return region;
let mut subst_regions = vec![self.universal_regions.fr_static];
let universal_substs = infcx.tcx.fold_regions(substs, &mut false, |region, _| {
if let ty::RePlaceholder(..) = region.kind() {
// Higher kinded regions don't need remapping, they don't refer to anything outside of this the substs.
return region;
}
let vid = self.to_region_vid(region);
trace!(?vid);
let scc = self.constraint_sccs.scc(vid);
trace!(?scc);
match self.scc_values.universal_regions_outlived_by(scc).find_map(|lb| {
self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?)
}) {
Some(region) => {
let vid = self.universal_regions.to_region_vid(region);
subst_regions.push(vid);
region
}
let vid = self.to_region_vid(region);
trace!(?vid);
let scc = self.constraint_sccs.scc(vid);
trace!(?scc);
match self.scc_values.universal_regions_outlived_by(scc).find_map(|lb| {
self.eval_equal(vid, lb).then_some(self.definitions[lb].external_name?)
}) {
Some(region) => {
let vid = self.universal_regions.to_region_vid(region);
subst_regions.push(vid);
region
}
None => {
subst_regions.push(vid);
infcx.tcx.sess.delay_span_bug(
concrete_type.span,
"opaque type with non-universal region substs",
);
infcx.tcx.lifetimes.re_static
}
None => {
subst_regions.push(vid);
infcx.tcx.sess.delay_span_bug(
concrete_type.span,
"opaque type with non-universal region substs",
);
infcx.tcx.lifetimes.re_static
}
});
}
});

subst_regions.sort();
subst_regions.dedup();
subst_regions.sort();
subst_regions.dedup();

let universal_concrete_type =
infcx.tcx.fold_regions(concrete_type, &mut false, |region, _| match *region {
ty::ReVar(vid) => subst_regions
.iter()
.find(|ur_vid| self.eval_equal(vid, **ur_vid))
.and_then(|ur_vid| self.definitions[*ur_vid].external_name)
.unwrap_or(infcx.tcx.lifetimes.re_root_empty),
_ => region,
});
let universal_concrete_type =
infcx.tcx.fold_regions(concrete_type, &mut false, |region, _| match *region {
ty::ReVar(vid) => subst_regions
.iter()
.find(|ur_vid| self.eval_equal(vid, **ur_vid))
.and_then(|ur_vid| self.definitions[*ur_vid].external_name)
.unwrap_or(infcx.tcx.lifetimes.re_root_empty),
_ => region,
});

debug!(?universal_concrete_type, ?universal_substs);
debug!(?universal_concrete_type, ?universal_substs);

let opaque_type_key =
OpaqueTypeKey { def_id: opaque_type_key.def_id, substs: universal_substs };
let remapped_type = infcx.infer_opaque_definition_from_instantiation(
opaque_type_key,
universal_concrete_type,
);
let ty = if check_opaque_type_parameter_valid(
infcx.tcx,
opaque_type_key,
origin,
concrete_type.span,
) {
remapped_type
} else {
infcx.tcx.ty_error()
};
(opaque_type_key, OpaqueHiddenType { ty, span: concrete_type.span })
})
.collect()
let opaque_type_key =
OpaqueTypeKey { def_id: opaque_type_key.def_id, substs: universal_substs };
let remapped_type = infcx.infer_opaque_definition_from_instantiation(
opaque_type_key,
universal_concrete_type,
);
let ty = if check_opaque_type_parameter_valid(
infcx.tcx,
opaque_type_key,
origin,
concrete_type.span,
) {
remapped_type
} else {
infcx.tcx.ty_error()
};
// Sometimes two opaque types are the same only after we remap the generic parameters
// back to the opaque type definition. E.g. we may have `OpaqueType<X, Y>` mapped to `(X, Y)`
// and `OpaqueType<Y, X>` mapped to `(Y, X)`, and those are the same, but we only know that
// once we convert the generic parameters to those of the opaque type.
if let Some(prev) = result.get_mut(&opaque_type_key) {
if prev.ty != ty {
let mut err = infcx.tcx.sess.struct_span_err(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this an error that we can trigger from code? If so, could you add a unit test (if it's easy to reproduce)?

Otherwise, maybe we can bug (or delay a bug) here instead if it should never happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should with type-alias-impl-trait, but due to a funky bug it never happens in practice. I'll prep a second PR that we won't backport that does this.

Until then, I'll leave in this error just so in case I'm wrong, we get an error instead of an ICE on beta.

concrete_type.span,
&format!("hidden type `{}` differed from previous `{}`", ty, prev.ty),
);
err.span_note(prev.span, "previous hidden type bound here");
err.emit();
prev.ty = infcx.tcx.ty_error();
}
// Pick a better span if there is one.
// FIXME(oli-obk): collect multiple spans for better diagnostics down the road.
prev.span = prev.span.substitute_dummy(concrete_type.span);
} else {
result.insert(opaque_type_key, OpaqueHiddenType { ty, span: concrete_type.span });
}
}
result
}

/// Map the regions in the type to named regions. This is similar to what
Expand Down
30 changes: 30 additions & 0 deletions src/test/ui/type-alias-impl-trait/multiple_definitions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// check-pass

use std::marker::PhantomData;

pub struct ConcreteError {}
pub trait IoBase {}
struct X {}
impl IoBase for X {}

pub struct ClusterIterator<B, E, S = B> {
pub fat: B,
phantom_s: PhantomData<S>,
phantom_e: PhantomData<E>,
}

pub struct FileSystem<IO: IoBase> {
pub disk: IO,
}

impl<IO: IoBase> FileSystem<IO> {
pub fn cluster_iter(&self) -> ClusterIterator<impl IoBase + '_, ConcreteError> {
ClusterIterator {
fat: X {},
phantom_s: PhantomData::default(),
phantom_e: PhantomData::default(),
}
}
}

fn main() {}