From aa00e3a552d453f8af0d033d6207a0e09d159d2e Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Thu, 12 May 2016 14:19:26 -0400 Subject: [PATCH] re-introduce a cache for ast-ty-to-ty It turns out that `ast_ty_to_ty` is supposed to be updating the `def` after it finishes, but at some point in the past it stopped doing so. This was never noticed because of the `ast_ty_to_ty_cache`, but that cache was recently removed. This PR fixes the code to update the def properly, but apparently that is not quite enough to make the operation idempotent, so for now we reintroduce the cache too. Fixes #33425. --- src/librustc_resolve/lib.rs | 2 + src/librustc_typeck/astconv.rs | 58 +++++++++++++------ src/librustc_typeck/check/mod.rs | 21 ++++--- src/librustc_typeck/collect.rs | 12 +++- src/librustc_typeck/lib.rs | 6 +- .../associated-types-in-bound-type-arg.rs | 26 +++++++++ 6 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 src/test/run-pass/associated-types-in-bound-type-arg.rs diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index fc048c86dc9dd..9cec3a5179496 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -2544,6 +2544,8 @@ impl<'a> Resolver<'a> { /// returned value. See `hir::def::PathResolution` for more info. fn resolve_path(&mut self, id: NodeId, path: &Path, path_depth: usize, namespace: Namespace) -> Result { + debug!("resolve_path(id={:?} path={:?}, path_depth={:?})", id, path, path_depth); + let span = path.span; let segments = &path.segments[..path.segments.len() - path_depth]; diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 725d40889ceec..c8e247fb9181c 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -64,10 +64,10 @@ use rscope::{self, UnelidableRscope, RegionScope, ElidableRscope, ObjectLifetimeDefaultRscope, ShiftedRscope, BindingRscope, ElisionFailureInfo, ElidedLifetime}; use util::common::{ErrorReported, FN_OUTPUT_NAME}; -use util::nodemap::FnvHashSet; +use util::nodemap::{NodeMap, FnvHashSet}; use rustc_const_math::ConstInt; - +use std::cell::RefCell; use syntax::{abi, ast}; use syntax::codemap::{Span, Pos}; use syntax::errors::DiagnosticBuilder; @@ -81,6 +81,9 @@ use rustc_back::slice; pub trait AstConv<'gcx, 'tcx> { fn tcx<'a>(&'a self) -> TyCtxt<'a, 'gcx, 'tcx>; + /// A cache used for the result of `ast_ty_to_ty_cache` + fn ast_ty_to_ty_cache(&self) -> &RefCell>>; + /// Identify the type scheme for an item with a type, like a type /// alias, fn, or struct. This allows you to figure out the set of /// type parameters defined on the item. @@ -1416,13 +1419,16 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { rscope: &RegionScope, span: Span, param_mode: PathParamMode, - def: &Def, + def: Def, opt_self_ty: Option>, base_segments: &[hir::PathSegment]) -> Ty<'tcx> { let tcx = self.tcx(); - match *def { + debug!("base_def_to_ty(def={:?}, opt_self_ty={:?}, base_segments={:?})", + def, opt_self_ty, base_segments); + + match def { Def::Trait(trait_def_id) => { // N.B. this case overlaps somewhat with // TyObjectSum, see that fn for details @@ -1515,20 +1521,27 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { rscope: &RegionScope, span: Span, param_mode: PathParamMode, - def: &Def, + mut def: Def, opt_self_ty: Option>, base_segments: &[hir::PathSegment], assoc_segments: &[hir::PathSegment]) - -> Ty<'tcx> { + -> (Ty<'tcx>, Def) { + debug!("finish_resolving_def_to_ty(def={:?}, \ + base_segments={:?}, \ + assoc_segments={:?})", + def, + base_segments, + assoc_segments); let mut ty = self.base_def_to_ty(rscope, span, param_mode, def, opt_self_ty, base_segments); - let mut def = *def; + debug!("finish_resolving_def_to_ty: base_def_to_ty returned {:?}", ty); // If any associated type segments remain, attempt to resolve them. for segment in assoc_segments { + debug!("finish_resolving_def_to_ty: segment={:?}", segment); if ty.sty == ty::TyError { break; } @@ -1540,7 +1553,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { ty = a_ty; def = a_def; } - ty + (ty, def) } /// Parses the programmer's textual representation of a type into our @@ -1551,7 +1564,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { let tcx = self.tcx(); - match ast_ty.node { + let cache = self.ast_ty_to_ty_cache(); + match cache.borrow().get(&ast_ty.id) { + Some(ty) => { return ty; } + None => { } + } + + let result_ty = match ast_ty.node { hir::TyVec(ref ty) => { tcx.mk_slice(self.ast_ty_to_ty(rscope, &ty)) } @@ -1599,6 +1618,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { self.conv_ty_poly_trait_ref(rscope, ast_ty.span, bounds) } hir::TyPath(ref maybe_qself, ref path) => { + debug!("ast_ty_to_ty: maybe_qself={:?} path={:?}", maybe_qself, path); let path_res = if let Some(&d) = tcx.def_map.borrow().get(&ast_ty.id) { d } else if let Some(hir::QSelf { position: 0, .. }) = *maybe_qself { @@ -1615,13 +1635,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { let opt_self_ty = maybe_qself.as_ref().map(|qself| { self.ast_ty_to_ty(rscope, &qself.ty) }); - let ty = self.finish_resolving_def_to_ty(rscope, - ast_ty.span, - PathParamMode::Explicit, - &def, - opt_self_ty, - &path.segments[..base_ty_end], - &path.segments[base_ty_end..]); + let (ty, _def) = self.finish_resolving_def_to_ty(rscope, + ast_ty.span, + PathParamMode::Explicit, + def, + opt_self_ty, + &path.segments[..base_ty_end], + &path.segments[base_ty_end..]); if path_res.depth != 0 && ty.sty != ty::TyError { // Write back the new resolution. @@ -1675,7 +1695,11 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { // handled specially and will not descend into this routine. self.ty_infer(None, None, None, ast_ty.span) } - } + }; + + cache.borrow_mut().insert(ast_ty.id, result_ty); + + result_ty } pub fn ty_of_arg(&self, diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f428023da9b82..6dfad749982b6 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -346,6 +346,8 @@ impl UnsafetyState { #[derive(Clone)] pub struct FnCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { + ast_ty_to_ty_cache: RefCell>>, + body_id: ast::NodeId, // This flag is set to true if, during the writeback phase, we encounter @@ -1262,6 +1264,10 @@ pub fn check_enum_variants<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, impl<'a, 'gcx, 'tcx> AstConv<'gcx, 'tcx> for FnCtxt<'a, 'gcx, 'tcx> { fn tcx<'b>(&'b self) -> TyCtxt<'b, 'gcx, 'tcx> { self.tcx } + fn ast_ty_to_ty_cache(&self) -> &RefCell>> { + &self.ast_ty_to_ty_cache + } + fn get_item_type_scheme(&self, _: Span, id: DefId) -> Result, ErrorReported> { @@ -1434,6 +1440,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { body_id: ast::NodeId) -> FnCtxt<'a, 'gcx, 'tcx> { FnCtxt { + ast_ty_to_ty_cache: RefCell::new(NodeMap()), body_id: body_id, writeback_errors: Cell::new(false), err_count_on_creation: inh.tcx.sess.err_count(), @@ -3845,15 +3852,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { if path_res.depth == 0 { Some((opt_self_ty, &path.segments, path_res.base_def)) } else { - let mut def = path_res.base_def; + let def = path_res.base_def; let ty_segments = path.segments.split_last().unwrap().1; let base_ty_end = path.segments.len() - path_res.depth; - let ty = AstConv::finish_resolving_def_to_ty(self, self, span, - PathParamMode::Optional, - &mut def, - opt_self_ty, - &ty_segments[..base_ty_end], - &ty_segments[base_ty_end..]); + let (ty, _def) = AstConv::finish_resolving_def_to_ty(self, self, span, + PathParamMode::Optional, + def, + opt_self_ty, + &ty_segments[..base_ty_end], + &ty_segments[base_ty_end..]); let item_segment = path.segments.last().unwrap(); let item_name = item_segment.identifier.name; let def = match self.resolve_ufcs(span, item_name, ty, node_id) { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 797a1509ebee5..f9a22e2a577b4 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -78,11 +78,12 @@ use rscope::*; use rustc::dep_graph::DepNode; use rustc::hir::map as hir_map; use util::common::{ErrorReported, MemoizationMap}; -use util::nodemap::FnvHashMap; +use util::nodemap::{NodeMap, FnvHashMap}; use {CrateCtxt, write_ty_to_tcx}; use rustc_const_math::ConstInt; +use std::cell::RefCell; use std::collections::HashSet; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::rc::Rc; @@ -146,7 +147,10 @@ impl<'a, 'tcx, 'v> intravisit::Visitor<'v> for CollectItemTypesVisitor<'a, 'tcx> impl<'a,'tcx> CrateCtxt<'a,'tcx> { fn icx(&'a self, param_bounds: &'a GetTypeParameterBounds<'tcx>) -> ItemCtxt<'a,'tcx> { - ItemCtxt { ccx: self, param_bounds: param_bounds } + ItemCtxt { + ccx: self, + param_bounds: param_bounds, + } } fn cycle_check(&self, @@ -298,6 +302,10 @@ impl<'a,'tcx> ItemCtxt<'a,'tcx> { impl<'a, 'tcx> AstConv<'tcx, 'tcx> for ItemCtxt<'a, 'tcx> { fn tcx<'b>(&'b self) -> TyCtxt<'b, 'tcx, 'tcx> { self.ccx.tcx } + fn ast_ty_to_ty_cache(&self) -> &RefCell>> { + &self.ccx.ast_ty_to_ty_cache + } + fn get_item_type_scheme(&self, span: Span, id: DefId) -> Result, ErrorReported> { diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index b88b3c9802d30..f41da95c13537 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -116,6 +116,7 @@ use syntax::ast; use syntax::abi::Abi; use std::cell::RefCell; +use util::nodemap::NodeMap; // NB: This module needs to be declared first so diagnostics are // registered before they are used. @@ -136,7 +137,9 @@ pub struct TypeAndSubsts<'tcx> { } pub struct CrateCtxt<'a, 'tcx: 'a> { - // A mapping from method call sites to traits that have that method. + ast_ty_to_ty_cache: RefCell>>, + + /// A mapping from method call sites to traits that have that method. pub trait_map: hir::TraitMap, /// A vector of every trait accessible in the whole crate @@ -334,6 +337,7 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, -> CompileResult { let time_passes = tcx.sess.time_passes(); let ccx = CrateCtxt { + ast_ty_to_ty_cache: RefCell::new(NodeMap()), trait_map: trait_map, all_traits: RefCell::new(None), stack: RefCell::new(Vec::new()), diff --git a/src/test/run-pass/associated-types-in-bound-type-arg.rs b/src/test/run-pass/associated-types-in-bound-type-arg.rs new file mode 100644 index 0000000000000..18803d15719e8 --- /dev/null +++ b/src/test/run-pass/associated-types-in-bound-type-arg.rs @@ -0,0 +1,26 @@ +// Copyright 2012 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test the case where we resolve `C::Result` and the trait bound +// itself includes a `Self::Item` shorthand. +// +// Regression test for issue #33425. + +trait ParallelIterator { + type Item; + fn drive_unindexed(self, consumer: C) -> C::Result + where C: Consumer; +} + +pub trait Consumer { + type Result; +} + +fn main() { }