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 hidden types in dead code #102700

Merged
merged 7 commits into from
Oct 14, 2022
Merged

Check hidden types in dead code #102700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5d15beb
Unconditionally encode hidden types in typeck results
oli-obk Sep 20, 2022
d643ae1
Remove some dead code
oli-obk Sep 27, 2022
9eb69e8
Move ReverseMapper logic onto OpaqueHiddenType
oli-obk Sep 27, 2022
70d39ab
Remap hidden types from typeck before storing them in the TypeckResult
oli-obk Sep 27, 2022
f85d3a7
Check hidden types in dead code
oli-obk Oct 1, 2022
e07f36c
Add a regression test
oli-obk Oct 5, 2022
3c8b46c
typeck result is now affected by opaque type spans
oli-obk Oct 5, 2022
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
255 changes: 7 additions & 248 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 @@ -2,12 +2,10 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::vec_map::VecMap;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::OpaqueTyOrigin;
use rustc_infer::infer::error_reporting::unexpected_hidden_region_diagnostic;
use rustc_infer::infer::TyCtxtInferExt as _;
use rustc_infer::infer::{DefiningAnchor, InferCtxt};
use rustc_infer::traits::{Obligation, ObligationCause, TraitEngine};
use rustc_middle::ty::fold::{TypeFolder, TypeSuperFoldable};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts};
use rustc_middle::ty::subst::{GenericArgKind, InternalSubsts};
use rustc_middle::ty::visit::TypeVisitable;
use rustc_middle::ty::{
self, OpaqueHiddenType, OpaqueTypeKey, ToPredicate, Ty, TyCtxt, TypeFoldable,
Expand All @@ -16,8 +14,6 @@ use rustc_span::Span;
use rustc_trait_selection::traits::error_reporting::TypeErrCtxtExt as _;
use rustc_trait_selection::traits::TraitEngineExt as _;

use crate::session_diagnostics::ConstNotUsedTraitAlias;

use super::RegionInferenceContext;

impl<'tcx> RegionInferenceContext<'tcx> {
Expand Down Expand Up @@ -229,31 +225,9 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
return self.tcx.ty_error();
}

let OpaqueTypeKey { def_id, substs } = opaque_type_key;

// Use substs to build up a reverse map from regions to their
// identity mappings. This is necessary because of `impl
// Trait` lifetimes are computed by replacing existing
// lifetimes with 'static and remapping only those used in the
// `impl Trait` return type, resulting in the parameters
// shifting.
let id_substs = InternalSubsts::identity_for_item(self.tcx, def_id.to_def_id());
debug!(?id_substs);
let map: FxHashMap<GenericArg<'tcx>, GenericArg<'tcx>> =
substs.iter().enumerate().map(|(index, subst)| (subst, id_substs[index])).collect();
debug!("map = {:#?}", map);

// Convert the type from the function into a type valid outside
// the function, by replacing invalid regions with 'static,
// after producing an error for each of them.
let definition_ty = instantiated_ty.ty.fold_with(&mut ReverseMapper::new(
self.tcx,
opaque_type_key,
map,
instantiated_ty.ty,
instantiated_ty.span,
));
debug!(?definition_ty);
let definition_ty = instantiated_ty
.remap_generic_params_to_declaration_params(opaque_type_key, self.tcx, false)
.ty;

if !check_opaque_type_parameter_valid(
self.tcx,
Expand All @@ -269,6 +243,7 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
let OpaqueTyOrigin::TyAlias = origin else {
return definition_ty;
};
let def_id = opaque_type_key.def_id;
// This logic duplicates most of `check_opaque_meets_bounds`.
// FIXME(oli-obk): Also do region checks here and then consider removing `check_opaque_meets_bounds` entirely.
let param_env = self.tcx.param_env(def_id);
Expand All @@ -284,6 +259,8 @@ impl<'tcx> InferCtxtExt<'tcx> for InferCtxt<'tcx> {
.to_predicate(infcx.tcx);
let mut fulfillment_cx = <dyn TraitEngine<'tcx>>::new(infcx.tcx);

let id_substs = InternalSubsts::identity_for_item(self.tcx, def_id.to_def_id());

// Require that the hidden type actually fulfills all the bounds of the opaque type, even without
// the bounds that the function supplies.
match infcx.register_hidden_type(
Expand Down Expand Up @@ -424,221 +401,3 @@ fn check_opaque_type_parameter_valid(
}
true
}

struct ReverseMapper<'tcx> {
tcx: TyCtxt<'tcx>,

key: ty::OpaqueTypeKey<'tcx>,
map: FxHashMap<GenericArg<'tcx>, GenericArg<'tcx>>,
do_not_error: bool,

/// initially `Some`, set to `None` once error has been reported
hidden_ty: Option<Ty<'tcx>>,

/// Span of function being checked.
span: Span,
}

impl<'tcx> ReverseMapper<'tcx> {
fn new(
tcx: TyCtxt<'tcx>,
key: ty::OpaqueTypeKey<'tcx>,
map: FxHashMap<GenericArg<'tcx>, GenericArg<'tcx>>,
hidden_ty: Ty<'tcx>,
span: Span,
) -> Self {
Self { tcx, key, map, do_not_error: false, hidden_ty: Some(hidden_ty), span }
}

fn fold_kind_no_missing_regions_error(&mut self, kind: GenericArg<'tcx>) -> GenericArg<'tcx> {
assert!(!self.do_not_error);
self.do_not_error = true;
let kind = kind.fold_with(self);
self.do_not_error = false;
kind
}

fn fold_kind_normally(&mut self, kind: GenericArg<'tcx>) -> GenericArg<'tcx> {
assert!(!self.do_not_error);
kind.fold_with(self)
}
}

impl<'tcx> TypeFolder<'tcx> for ReverseMapper<'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

#[instrument(skip(self), level = "debug")]
fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
match *r {
// Ignore bound regions and `'static` regions that appear in the
// type, we only need to remap regions that reference lifetimes
// from the function declaration.
// This would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
ty::ReLateBound(..) | ty::ReStatic => return r,

// If regions have been erased (by writeback), don't try to unerase
// them.
ty::ReErased => return r,

// The regions that we expect from borrow checking.
ty::ReEarlyBound(_) | ty::ReFree(_) => {}

ty::RePlaceholder(_) | ty::ReVar(_) => {
// All of the regions in the type should either have been
// erased by writeback, or mapped back to named regions by
// borrow checking.
bug!("unexpected region kind in opaque type: {:?}", r);
}
}

let generics = self.tcx().generics_of(self.key.def_id);
match self.map.get(&r.into()).map(|k| k.unpack()) {
Some(GenericArgKind::Lifetime(r1)) => r1,
Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
None if self.do_not_error => self.tcx.lifetimes.re_static,
None if generics.parent.is_some() => {
if let Some(hidden_ty) = self.hidden_ty.take() {
Copy link
Member

Choose a reason for hiding this comment

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

Why did this code exist? What did it achieve?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, well it was dead code anyways. I should read the commit title.

unexpected_hidden_region_diagnostic(
self.tcx,
self.tcx.def_span(self.key.def_id),
hidden_ty,
r,
self.key,
)
.emit();
}
self.tcx.lifetimes.re_static
}
None => {
self.tcx
.sess
.struct_span_err(self.span, "non-defining opaque type use in defining scope")
.span_label(
self.span,
format!(
"lifetime `{}` is part of concrete type but not used in \
parameter list of the `impl Trait` type alias",
r
),
)
.emit();

self.tcx().lifetimes.re_static
}
}
}

fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
match *ty.kind() {
ty::Closure(def_id, substs) => {
// I am a horrible monster and I pray for death. When
// we encounter a closure here, it is always a closure
// from within the function that we are currently
// type-checking -- one that is now being encapsulated
// in an opaque type. Ideally, we would
// go through the types/lifetimes that it references
// and treat them just like we would any other type,
// which means we would error out if we find any
// reference to a type/region that is not in the
// "reverse map".
//
// **However,** in the case of closures, there is a
// somewhat subtle (read: hacky) consideration. The
// problem is that our closure types currently include
// all the lifetime parameters declared on the
// enclosing function, even if they are unused by the
// closure itself. We can't readily filter them out,
// so here we replace those values with `'empty`. This
// can't really make a difference to the rest of the
// compiler; those regions are ignored for the
// outlives relation, and hence don't affect trait
// selection or auto traits, and they are erased
// during codegen.

let generics = self.tcx.generics_of(def_id);
let substs = self.tcx.mk_substs(substs.iter().enumerate().map(|(index, kind)| {
if index < generics.parent_count {
// Accommodate missing regions in the parent kinds...
self.fold_kind_no_missing_regions_error(kind)
} else {
// ...but not elsewhere.
self.fold_kind_normally(kind)
}
}));

self.tcx.mk_closure(def_id, substs)
}

ty::Generator(def_id, substs, movability) => {
let generics = self.tcx.generics_of(def_id);
let substs = self.tcx.mk_substs(substs.iter().enumerate().map(|(index, kind)| {
if index < generics.parent_count {
// Accommodate missing regions in the parent kinds...
self.fold_kind_no_missing_regions_error(kind)
} else {
// ...but not elsewhere.
self.fold_kind_normally(kind)
}
}));

self.tcx.mk_generator(def_id, substs, movability)
}

ty::Param(param) => {
// Look it up in the substitution list.
match self.map.get(&ty.into()).map(|k| k.unpack()) {
// Found it in the substitution list; replace with the parameter from the
// opaque type.
Some(GenericArgKind::Type(t1)) => t1,
Some(u) => panic!("type mapped to unexpected kind: {:?}", u),
None => {
debug!(?param, ?self.map);
self.tcx
.sess
.struct_span_err(
self.span,
&format!(
"type parameter `{}` is part of concrete type but not \
used in parameter list for the `impl Trait` type alias",
ty
),
)
.emit();

self.tcx().ty_error()
}
}
}

_ => ty.super_fold_with(self),
}
}

fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> {
trace!("checking const {:?}", ct);
// Find a const parameter
match ct.kind() {
ty::ConstKind::Param(..) => {
// Look it up in the substitution list.
match self.map.get(&ct.into()).map(|k| k.unpack()) {
// Found it in the substitution list, replace with the parameter from the
// opaque type.
Some(GenericArgKind::Const(c1)) => c1,
Some(u) => panic!("const mapped to unexpected kind: {:?}", u),
None => {
self.tcx.sess.emit_err(ConstNotUsedTraitAlias {
ct: ct.to_string(),
span: self.span,
});

self.tcx().const_error(ct.ty())
}
}
}

_ => ct,
}
}
}
9 changes: 0 additions & 9 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,6 @@ pub(crate) struct VarNeedNotMut {
#[suggestion_short(applicability = "machine-applicable", code = "")]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(borrowck::const_not_used_in_type_alias)]
pub(crate) struct ConstNotUsedTraitAlias {
pub ct: String,
#[primary_span]
pub span: Span,
}

#[derive(Diagnostic)]
#[diag(borrowck::var_cannot_escape_closure)]
#[note]
Expand Down
51 changes: 27 additions & 24 deletions compiler/rustc_hir_analysis/src/check/writeback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,33 +536,36 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
let opaque_types =
self.fcx.infcx.inner.borrow_mut().opaque_type_storage.take_opaque_types();
for (opaque_type_key, decl) in opaque_types {
let hidden_type = match decl.origin {
hir::OpaqueTyOrigin::FnReturn(_) | hir::OpaqueTyOrigin::AsyncFn(_) => {
let ty = self.resolve(decl.hidden_type.ty, &decl.hidden_type.span);
struct RecursionChecker {
def_id: LocalDefId,
}
impl<'tcx> ty::TypeVisitor<'tcx> for RecursionChecker {
type BreakTy = ();
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
if let ty::Opaque(def_id, _) = *t.kind() {
if def_id == self.def_id.to_def_id() {
return ControlFlow::Break(());
}
}
t.super_visit_with(self)
let hidden_type = self.resolve(decl.hidden_type, &decl.hidden_type.span);
let opaque_type_key = self.resolve(opaque_type_key, &decl.hidden_type.span);

struct RecursionChecker {
def_id: LocalDefId,
}
impl<'tcx> ty::TypeVisitor<'tcx> for RecursionChecker {
type BreakTy = ();
fn visit_ty(&mut self, t: Ty<'tcx>) -> ControlFlow<Self::BreakTy> {
if let ty::Opaque(def_id, _) = *t.kind() {
if def_id == self.def_id.to_def_id() {
return ControlFlow::Break(());
}
}
if ty
.visit_with(&mut RecursionChecker { def_id: opaque_type_key.def_id })
.is_break()
{
return;
}
Some(ty)
t.super_visit_with(self)
}
hir::OpaqueTyOrigin::TyAlias => None,
};
}
if hidden_type
.visit_with(&mut RecursionChecker { def_id: opaque_type_key.def_id })
.is_break()
{
continue;
}

let hidden_type = hidden_type.remap_generic_params_to_declaration_params(
opaque_type_key,
self.fcx.infcx.tcx,
true,
);

self.typeck_results.concrete_opaque_types.insert(opaque_type_key.def_id, hidden_type);
}
}
Expand Down
Loading