Skip to content

Commit

Permalink
Rollup merge of #33596 - nikomatsakis:issue-33586-regr-assoc-type-pat…
Browse files Browse the repository at this point in the history
…h, r=eddyb

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.

r? @eddyb
  • Loading branch information
eddyb committed May 12, 2016
2 parents 377dced + aa00e3a commit dbd5968
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 27 deletions.
2 changes: 2 additions & 0 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2542,6 +2542,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<PathResolution, bool /* true if an error was reported */ > {
debug!("resolve_path(id={:?} path={:?}, path_depth={:?})", id, path, path_depth);

let span = path.span;
let segments = &path.segments[..path.segments.len() - path_depth];

Expand Down
58 changes: 41 additions & 17 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<NodeMap<Ty<'tcx>>>;

/// 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.
Expand Down Expand Up @@ -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<Ty<'tcx>>,
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
Expand Down Expand Up @@ -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<Ty<'tcx>>,
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;
}
Expand All @@ -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
Expand All @@ -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))
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
21 changes: 14 additions & 7 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,8 @@ impl UnsafetyState {

#[derive(Clone)]
pub struct FnCtxt<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
ast_ty_to_ty_cache: RefCell<NodeMap<Ty<'tcx>>>,

body_id: ast::NodeId,

// This flag is set to true if, during the writeback phase, we encounter
Expand Down Expand Up @@ -1263,6 +1265,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<NodeMap<Ty<'tcx>>> {
&self.ast_ty_to_ty_cache
}

fn get_item_type_scheme(&self, _: Span, id: DefId)
-> Result<ty::TypeScheme<'tcx>, ErrorReported>
{
Expand Down Expand Up @@ -1435,6 +1441,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(),
Expand Down Expand Up @@ -3852,15 +3859,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) {
Expand Down
12 changes: 10 additions & 2 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<F,R>(&self,
Expand Down Expand Up @@ -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<NodeMap<Ty<'tcx>>> {
&self.ccx.ast_ty_to_ty_cache
}

fn get_item_type_scheme(&self, span: Span, id: DefId)
-> Result<ty::TypeScheme<'tcx>, ErrorReported>
{
Expand Down
6 changes: 5 additions & 1 deletion src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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<NodeMap<Ty<'tcx>>>,

/// 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
Expand Down Expand Up @@ -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()),
Expand Down
26 changes: 26 additions & 0 deletions src/test/run-pass/associated-types-in-bound-type-arg.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<C>(self, consumer: C) -> C::Result
where C: Consumer<Self::Item>;
}

pub trait Consumer<ITEM> {
type Result;
}

fn main() { }

0 comments on commit dbd5968

Please sign in to comment.