Skip to content

Commit

Permalink
Auto merge of #70164 - eddyb:walk-generic-arg, r=nikomatsakis
Browse files Browse the repository at this point in the history
ty/walk: iterate `GenericArg`s instead of `Ty`s.

Before this PR, `Ty::walk` only iterated over `Ty`s, but that's becoming an increasing problem with `const` generics, as `ty::Const`s in `Substs` are missed by it.

By working with `GenericArg` instead, we can handle both `Ty`s and `ty::Const`s, but also `ty::Region`s, which used to require ad-hoc mechanisms such as `push_regions`.

I've also removed `TraitRef::input_types`, as it's both long obsolete, and easy to misuse.
  • Loading branch information
bors committed Apr 7, 2020
2 parents bf1f2ee + 626abc7 commit 209b2be
Show file tree
Hide file tree
Showing 23 changed files with 618 additions and 603 deletions.
113 changes: 64 additions & 49 deletions src/librustc_infer/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,52 +7,59 @@ use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{Body, Expr, ExprKind, FnRetTy, HirId, Local, Pat};
use rustc_middle::hir::map::Map;
use rustc_middle::ty::print::Print;
use rustc_middle::ty::{self, DefIdTree, Infer, Ty, TyVar};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind};
use rustc_middle::ty::{self, DefIdTree, Ty};
use rustc_span::source_map::DesugaringKind;
use rustc_span::symbol::kw;
use rustc_span::Span;
use std::borrow::Cow;

struct FindLocalByTypeVisitor<'a, 'tcx> {
struct FindHirNodeVisitor<'a, 'tcx> {
infcx: &'a InferCtxt<'a, 'tcx>,
target_ty: Ty<'tcx>,
hir_map: Map<'tcx>,
target: GenericArg<'tcx>,
found_node_ty: Option<Ty<'tcx>>,
found_local_pattern: Option<&'tcx Pat<'tcx>>,
found_arg_pattern: Option<&'tcx Pat<'tcx>>,
found_ty: Option<Ty<'tcx>>,
found_closure: Option<&'tcx ExprKind<'tcx>>,
found_closure: Option<&'tcx Expr<'tcx>>,
found_method_call: Option<&'tcx Expr<'tcx>>,
}

impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> {
fn new(infcx: &'a InferCtxt<'a, 'tcx>, target_ty: Ty<'tcx>, hir_map: Map<'tcx>) -> Self {
impl<'a, 'tcx> FindHirNodeVisitor<'a, 'tcx> {
fn new(infcx: &'a InferCtxt<'a, 'tcx>, target: GenericArg<'tcx>) -> Self {
Self {
infcx,
target_ty,
hir_map,
target,
found_node_ty: None,
found_local_pattern: None,
found_arg_pattern: None,
found_ty: None,
found_closure: None,
found_method_call: None,
}
}

fn node_matches_type(&mut self, hir_id: HirId) -> Option<Ty<'tcx>> {
fn node_ty_contains_target(&mut self, hir_id: HirId) -> Option<Ty<'tcx>> {
let ty_opt =
self.infcx.in_progress_tables.and_then(|tables| tables.borrow().node_type_opt(hir_id));
match ty_opt {
Some(ty) => {
let ty = self.infcx.resolve_vars_if_possible(&ty);
if ty.walk().any(|inner_ty| {
inner_ty == self.target_ty
|| match (&inner_ty.kind, &self.target_ty.kind) {
(&Infer(TyVar(a_vid)), &Infer(TyVar(b_vid))) => self
.infcx
.inner
.borrow_mut()
.type_variables
.sub_unified(a_vid, b_vid),
if ty.walk().any(|inner| {
inner == self.target
|| match (inner.unpack(), self.target.unpack()) {
(GenericArgKind::Type(inner_ty), GenericArgKind::Type(target_ty)) => {
match (&inner_ty.kind, &target_ty.kind) {
(
&ty::Infer(ty::TyVar(a_vid)),
&ty::Infer(ty::TyVar(b_vid)),
) => self
.infcx
.inner
.borrow_mut()
.type_variables
.sub_unified(a_vid, b_vid),
_ => false,
}
}
_ => false,
}
}) {
Expand All @@ -66,36 +73,39 @@ impl<'a, 'tcx> FindLocalByTypeVisitor<'a, 'tcx> {
}
}

impl<'a, 'tcx> Visitor<'tcx> for FindLocalByTypeVisitor<'a, 'tcx> {
impl<'a, 'tcx> Visitor<'tcx> for FindHirNodeVisitor<'a, 'tcx> {
type Map = Map<'tcx>;

fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
NestedVisitorMap::OnlyBodies(self.hir_map)
NestedVisitorMap::OnlyBodies(self.infcx.tcx.hir())
}

fn visit_local(&mut self, local: &'tcx Local<'tcx>) {
if let (None, Some(ty)) = (self.found_local_pattern, self.node_matches_type(local.hir_id)) {
if let (None, Some(ty)) =
(self.found_local_pattern, self.node_ty_contains_target(local.hir_id))
{
self.found_local_pattern = Some(&*local.pat);
self.found_ty = Some(ty);
self.found_node_ty = Some(ty);
}
intravisit::walk_local(self, local);
}

fn visit_body(&mut self, body: &'tcx Body<'tcx>) {
for param in body.params {
if let (None, Some(ty)) = (self.found_arg_pattern, self.node_matches_type(param.hir_id))
if let (None, Some(ty)) =
(self.found_arg_pattern, self.node_ty_contains_target(param.hir_id))
{
self.found_arg_pattern = Some(&*param.pat);
self.found_ty = Some(ty);
self.found_node_ty = Some(ty);
}
}
intravisit::walk_body(self, body);
}

fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
if self.node_matches_type(expr.hir_id).is_some() {
if self.node_ty_contains_target(expr.hir_id).is_some() {
match expr.kind {
ExprKind::Closure(..) => self.found_closure = Some(&expr.kind),
ExprKind::Closure(..) => self.found_closure = Some(&expr),
ExprKind::MethodCall(..) => self.found_method_call = Some(&expr),
_ => {}
}
Expand Down Expand Up @@ -213,6 +223,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
(s, None, ty.prefix_string(), None, None)
}

// FIXME(eddyb) generalize all of this to handle `ty::Const` inference variables as well.
pub fn need_type_info_err(
&self,
body_id: Option<hir::BodyId>,
Expand All @@ -223,7 +234,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let ty = self.resolve_vars_if_possible(&ty);
let (name, name_sp, descr, parent_name, parent_descr) = self.extract_type_name(&ty, None);

let mut local_visitor = FindLocalByTypeVisitor::new(&self, ty, self.tcx.hir());
let mut local_visitor = FindHirNodeVisitor::new(&self, ty.into());
let ty_to_string = |ty: Ty<'tcx>| -> String {
let mut s = String::new();
let mut printer = ty::print::FmtPrinter::new(self.tcx, &mut s, Namespace::TypeNS);
Expand Down Expand Up @@ -276,7 +287,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
(!ty.is_impl_trait() || self.tcx.features().impl_trait_in_bindings)
};

let ty_msg = match local_visitor.found_ty {
let ty_msg = match local_visitor.found_node_ty {
Some(ty::TyS { kind: ty::Closure(_, substs), .. }) => {
let fn_sig = substs.as_closure().sig();
let args = closure_args(&fn_sig);
Expand Down Expand Up @@ -310,28 +321,32 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
error_code,
);

let suffix = match local_visitor.found_ty {
let suffix = match local_visitor.found_node_ty {
Some(ty::TyS { kind: ty::Closure(_, substs), .. }) => {
let fn_sig = substs.as_closure().sig();
let ret = fn_sig.output().skip_binder().to_string();

if let Some(ExprKind::Closure(_, decl, body_id, ..)) = local_visitor.found_closure {
if let Some(body) = self.tcx.hir().krate().bodies.get(body_id) {
closure_return_type_suggestion(
span,
&mut err,
&decl.output,
&body,
&descr,
&name,
&ret,
parent_name,
parent_descr,
);
// We don't want to give the other suggestions when the problem is the
// closure return type.
return err;
}
let closure_decl_and_body_id =
local_visitor.found_closure.and_then(|closure| match &closure.kind {
ExprKind::Closure(_, decl, body_id, ..) => Some((decl, *body_id)),
_ => None,
});

if let Some((decl, body_id)) = closure_decl_and_body_id {
closure_return_type_suggestion(
span,
&mut err,
&decl.output,
self.tcx.hir().body(body_id),
&descr,
&name,
&ret,
parent_name,
parent_descr,
);
// We don't want to give the other suggestions when the problem is the
// closure return type.
return err;
}

// This shouldn't be reachable, but just in case we leave a reasonable fallback.
Expand Down
42 changes: 24 additions & 18 deletions src/librustc_infer/infer/outlives/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ use crate::infer::{GenericKind, VerifyBound};
use crate::traits;
use rustc_data_structures::captures::Captures;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::subst::{InternalSubsts, Subst};
use rustc_middle::ty::subst::{GenericArg, GenericArgKind, InternalSubsts, Subst};
use rustc_middle::ty::{self, Ty, TyCtxt};

use smallvec::smallvec;

/// The `TypeOutlives` struct has the job of "lowering" a `T: 'a`
/// obligation into a series of `'a: 'b` constraints and "verifys", as
/// described on the module comment. The final constraints are emitted
Expand Down Expand Up @@ -44,7 +42,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
match ty.kind {
ty::Param(p) => self.param_bound(p),
ty::Projection(data) => self.projection_bound(data),
_ => self.recursive_type_bound(ty),
_ => self.recursive_bound(ty.into()),
}
}

Expand Down Expand Up @@ -144,25 +142,33 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {

// see the extensive comment in projection_must_outlive
let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs);
let recursive_bound = self.recursive_type_bound(ty);
let recursive_bound = self.recursive_bound(ty.into());

VerifyBound::AnyBound(env_bounds.chain(trait_bounds).collect()).or(recursive_bound)
}

fn recursive_type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> {
let mut bounds = ty.walk_shallow().map(|subty| self.type_bound(subty)).collect::<Vec<_>>();

let mut regions = smallvec![];
ty.push_regions(&mut regions);
regions.retain(|r| !r.is_late_bound()); // ignore late-bound regions
bounds.push(VerifyBound::AllBounds(
regions.into_iter().map(|r| VerifyBound::OutlivedBy(r)).collect(),
));

// remove bounds that must hold, since they are not interesting
bounds.retain(|b| !b.must_hold());
fn recursive_bound(&self, parent: GenericArg<'tcx>) -> VerifyBound<'tcx> {
let mut bounds = parent
.walk_shallow()
.filter_map(|child| match child.unpack() {
GenericArgKind::Type(ty) => Some(self.type_bound(ty)),
GenericArgKind::Lifetime(lt) => {
// Ignore late-bound regions.
if !lt.is_late_bound() { Some(VerifyBound::OutlivedBy(lt)) } else { None }
}
GenericArgKind::Const(_) => Some(self.recursive_bound(child)),
})
.filter(|bound| {
// Remove bounds that must hold, since they are not interesting.
!bound.must_hold()
});

if bounds.len() == 1 { bounds.pop().unwrap() } else { VerifyBound::AllBounds(bounds) }
match (bounds.next(), bounds.next()) {
(Some(first), None) => first,
(first, second) => {
VerifyBound::AllBounds(first.into_iter().chain(second).chain(bounds).collect())
}
}
}

/// Searches the environment for where-clauses like `G: 'a` where
Expand Down
13 changes: 8 additions & 5 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use rustc_hir::def_id::DefId;
use rustc_hir::{GenericParamKind, PatKind};
use rustc_hir::{HirIdSet, Node};
use rustc_middle::lint::LintDiagnosticBuilder;
use rustc_middle::ty::subst::GenericArgKind;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_session::lint::FutureIncompatibleInfo;
use rustc_span::edition::Edition;
Expand Down Expand Up @@ -104,11 +105,13 @@ declare_lint_pass!(BoxPointers => [BOX_POINTERS]);

impl BoxPointers {
fn check_heap_type(&self, cx: &LateContext<'_, '_>, span: Span, ty: Ty<'_>) {
for leaf_ty in ty.walk() {
if leaf_ty.is_box() {
cx.struct_span_lint(BOX_POINTERS, span, |lint| {
lint.build(&format!("type uses owned (Box type) pointers: {}", ty)).emit()
});
for leaf in ty.walk() {
if let GenericArgKind::Type(leaf_ty) = leaf.unpack() {
if leaf_ty.is_box() {
cx.struct_span_lint(BOX_POINTERS, span, |lint| {
lint.build(&format!("type uses owned (Box type) pointers: {}", ty)).emit()
});
}
}
}
}
Expand Down
14 changes: 0 additions & 14 deletions src/librustc_middle/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,20 +263,6 @@ where
// Region folder

impl<'tcx> TyCtxt<'tcx> {
/// Collects the free and escaping regions in `value` into `region_set`. Returns
/// whether any late-bound regions were skipped
pub fn collect_regions<T>(self, value: &T, region_set: &mut FxHashSet<ty::Region<'tcx>>) -> bool
where
T: TypeFoldable<'tcx>,
{
let mut have_bound_regions = false;
self.fold_regions(value, &mut have_bound_regions, |r, d| {
region_set.insert(self.mk_region(r.shifted_out_to_binder(d)));
r
});
have_bound_regions
}

/// Folds the escaping and free regions in `value` using `f`, and
/// sets `skipped_regions` to true if any late-bound region was found
/// and skipped.
Expand Down
Loading

0 comments on commit 209b2be

Please sign in to comment.