From 4fecccbd2e9760cdc082b5030ebf82911b7e3632 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 10 Nov 2017 10:01:22 +0100 Subject: [PATCH 1/9] Make RegionVid use newtype_index! Closes #45843 --- .../infer/lexical_region_resolve/mod.rs | 18 ++++++++-------- src/librustc/infer/region_constraints/mod.rs | 4 ++-- src/librustc/infer/unify_key.rs | 6 +++--- src/librustc/ty/sty.rs | 21 +++++-------------- src/librustc/util/ppaux.rs | 4 ++-- 5 files changed, 21 insertions(+), 32 deletions(-) diff --git a/src/librustc/infer/lexical_region_resolve/mod.rs b/src/librustc/infer/lexical_region_resolve/mod.rs index 0692d284d7c16..5a4f2157298b0 100644 --- a/src/librustc/infer/lexical_region_resolve/mod.rs +++ b/src/librustc/infer/lexical_region_resolve/mod.rs @@ -171,7 +171,7 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { for (r, vid) in seeds { // While all things transitively reachable in the graph // from the variable (`'0` in the example above). - let seed_index = NodeIndex(vid.index as usize); + let seed_index = NodeIndex(vid.index() as usize); for succ_index in graph.depth_traverse(seed_index, OUTGOING) { let succ_index = succ_index.0; @@ -512,16 +512,16 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { match *constraint { Constraint::VarSubVar(a_id, b_id) => { graph.add_edge( - NodeIndex(a_id.index as usize), - NodeIndex(b_id.index as usize), + NodeIndex(a_id.index() as usize), + NodeIndex(b_id.index() as usize), *constraint, ); } Constraint::RegSubVar(_, b_id) => { - graph.add_edge(dummy_source, NodeIndex(b_id.index as usize), *constraint); + graph.add_edge(dummy_source, NodeIndex(b_id.index() as usize), *constraint); } Constraint::VarSubReg(a_id, _) => { - graph.add_edge(NodeIndex(a_id.index as usize), dummy_sink, *constraint); + graph.add_edge(NodeIndex(a_id.index() as usize), dummy_sink, *constraint); } Constraint::RegSubReg(..) => { // this would be an edge from `dummy_source` to @@ -630,9 +630,9 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { let node_idx = state.stack.pop().unwrap(); // check whether we've visited this node on some previous walk - if dup_vec[node_idx.index as usize] == u32::MAX { - dup_vec[node_idx.index as usize] = orig_node_idx.index; - } else if dup_vec[node_idx.index as usize] != orig_node_idx.index { + if dup_vec[node_idx.index() as usize] == u32::MAX { + dup_vec[node_idx.index() as usize] = orig_node_idx.index() as u32; + } else if dup_vec[node_idx.index() as usize] != orig_node_idx.index() as u32 { state.dup_found = true; } @@ -659,7 +659,7 @@ impl<'cx, 'gcx, 'tcx> LexicalResolver<'cx, 'gcx, 'tcx> { ) { debug!("process_edges(source_vid={:?}, dir={:?})", source_vid, dir); - let source_node_index = NodeIndex(source_vid.index as usize); + let source_node_index = NodeIndex(source_vid.index() as usize); for (_, edge) in graph.adjacent_edges(source_node_index, dir) { match edge.data { Constraint::VarSubVar(from_vid, to_vid) => { diff --git a/src/librustc/infer/region_constraints/mod.rs b/src/librustc/infer/region_constraints/mod.rs index 096037ebe880c..72740dd40be29 100644 --- a/src/librustc/infer/region_constraints/mod.rs +++ b/src/librustc/infer/region_constraints/mod.rs @@ -16,7 +16,7 @@ use self::CombineMapType::*; use super::{MiscVariable, RegionVariableOrigin, SubregionOrigin}; use super::unify_key; -use rustc_data_structures::indexed_vec::IndexVec; +use rustc_data_structures::indexed_vec::{IndexVec, Idx}; use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_data_structures::unify::{self, UnificationTable}; use ty::{self, Ty, TyCtxt}; @@ -404,7 +404,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> { } AddVar(vid) => { self.var_origins.pop().unwrap(); - assert_eq!(self.var_origins.len(), vid.index as usize); + assert_eq!(self.var_origins.len(), vid.index() as usize); } AddConstraint(ref constraint) => { self.data.constraints.remove(constraint); diff --git a/src/librustc/infer/unify_key.rs b/src/librustc/infer/unify_key.rs index d7e3a53ff25c9..99b11794cc5b5 100644 --- a/src/librustc/infer/unify_key.rs +++ b/src/librustc/infer/unify_key.rs @@ -33,7 +33,7 @@ pub struct RegionVidKey { impl Combine for RegionVidKey { fn combine(&self, other: &RegionVidKey) -> RegionVidKey { - let min_vid = if self.min_vid.index < other.min_vid.index { + let min_vid = if self.min_vid.index() < other.min_vid.index() { self.min_vid } else { other.min_vid @@ -45,8 +45,8 @@ impl Combine for RegionVidKey { impl UnifyKey for ty::RegionVid { type Value = RegionVidKey; - fn index(&self) -> u32 { self.index } - fn from_index(i: u32) -> ty::RegionVid { ty::RegionVid { index: i } } + fn index(&self) -> u32 { self.0 } + fn from_index(i: u32) -> ty::RegionVid { ty::RegionVid(i) } fn tag(_: Option) -> &'static str { "RegionVid" } } diff --git a/src/librustc/ty/sty.rs b/src/librustc/ty/sty.rs index 7406fbf820893..9d393296c5b22 100644 --- a/src/librustc/ty/sty.rs +++ b/src/librustc/ty/sty.rs @@ -998,22 +998,11 @@ pub struct FloatVid { pub index: u32, } -#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Copy, PartialOrd, Ord)] -pub struct RegionVid { - pub index: u32, -} - -// FIXME: We could convert this to use `newtype_index!` -impl Idx for RegionVid { - fn new(value: usize) -> Self { - assert!(value < ::std::u32::MAX as usize); - RegionVid { index: value as u32 } - } - - fn index(self) -> usize { - self.index as usize - } -} +newtype_index!(RegionVid + { + pub idx + DEBUG_FORMAT = custom, + }); #[derive(Clone, Copy, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable, PartialOrd, Ord)] pub struct SkolemizedRegionVid { diff --git a/src/librustc/util/ppaux.rs b/src/librustc/util/ppaux.rs index 2c3a32b2d159e..9ff3d73f5c40e 100644 --- a/src/librustc/util/ppaux.rs +++ b/src/librustc/util/ppaux.rs @@ -726,7 +726,7 @@ define_print! { } } ty::ReVar(region_vid) if cx.identify_regions => { - write!(f, "'{}rv", region_vid.index) + write!(f, "'{}rv", region_vid.index()) } ty::ReScope(_) | ty::ReVar(_) | @@ -850,7 +850,7 @@ impl fmt::Debug for ty::FloatVid { impl fmt::Debug for ty::RegionVid { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "'_#{}r", self.index) + write!(f, "'_#{}r", self.index()) } } From 19c17360d96820f581e29790dccfd916eebcabf2 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Fri, 10 Nov 2017 00:07:32 -0500 Subject: [PATCH 2/9] Check rvalue aggregates during check_stmt in tycheck, add initial, (not passing) test --- src/librustc_mir/transform/type_check.rs | 86 +++++++++++++++++++ .../compile-fail/aggregate-rvalues-typeck.rs | 23 +++++ 2 files changed, 109 insertions(+) create mode 100644 src/test/compile-fail/aggregate-rvalues-typeck.rs diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index cc6b702090314..7a801d887fbb0 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -549,6 +549,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { terr ); } + self.check_rvalue(mir, rv, location); } StatementKind::SetDiscriminant { ref lvalue, @@ -1011,6 +1012,91 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } + fn aggregate_field_ty(&mut self, ak: &Box>, field: usize, location: Location) + -> Result, FieldAccessError> + { + let tcx = self.tcx(); + + let (variant, substs) = match **ak { + AggregateKind::Adt(def, variant, substs, _) => { // handle unions? + (&def.variants[variant], substs) + }, + AggregateKind::Closure(def_id, substs) => { + return match substs.upvar_tys(def_id, tcx).nth(field) { + Some(ty) => Ok(ty), + None => Err(FieldAccessError::OutOfRange { + field_count: substs.upvar_tys(def_id, tcx).count() + }), + } + }, + AggregateKind::Generator(def_id, substs, _) => { + if let Some(ty) = substs.upvar_tys(def_id, tcx).nth(field) { + return Ok(ty); + } + + return match substs.field_tys(def_id, tcx).nth(field) { + Some(ty) => Ok(ty), + None => Err(FieldAccessError::OutOfRange { + field_count: substs.field_tys(def_id, tcx).count() + 1 + }), + } + }, + AggregateKind::Array(ty) => { + return Ok(ty); + }, + AggregateKind::Tuple => { + unreachable!("This should have been covered in check_rvalues"); + }, + }; + + if let Some(field) = variant.fields.get(field) { + Ok(self.normalize(&field.ty(tcx, substs), location)) + } else { + Err(FieldAccessError::OutOfRange { field_count: variant.fields.len() }) + } + } + + #[allow(dead_code)] + fn check_rvalue(&mut self, mir: &Mir<'tcx>, rv: &Rvalue<'tcx>, location: Location) { + let tcx = self.tcx(); + match rv { + Rvalue::Aggregate(ref ak, ref ops) => { + match **ak { + // tuple rvalue field type is always the type of the op. Nothing to check here. + AggregateKind::Tuple => { }, + _ => { + for (i, op) in ops.iter().enumerate() { + let field_ty = if let Ok(field_ty) = self.aggregate_field_ty(ak, i, location) { + field_ty + } else { + // TODO(nashenas88) log span_mirbug terr?? + continue; + }; + let op_ty = match op { + Operand::Consume(lv) => lv.ty(mir, tcx).to_ty(tcx), + Operand::Constant(c) => c.ty, + }; + if let Err(_terr) = self.sub_types(op_ty, field_ty, location.at_successor_within_block()) { + // TODO(nashenas88) log span_mirbug terr?? + } + } + }, + } + }, + // FIXME: These other cases have to be implemented in future PRs + Rvalue::Use(..) | + Rvalue::Repeat(..) | + Rvalue::Ref(..) | + Rvalue::Len(..) | + Rvalue::Cast(..) | + Rvalue::BinaryOp(..) | + Rvalue::CheckedBinaryOp(..) | + Rvalue::UnaryOp(..) | + Rvalue::Discriminant(..) | + Rvalue::NullaryOp(..) => { } + } + } + fn typeck_mir(&mut self, mir: &Mir<'tcx>) { self.last_span = mir.span; debug!("run_on_mir: {:?}", mir.span); diff --git a/src/test/compile-fail/aggregate-rvalues-typeck.rs b/src/test/compile-fail/aggregate-rvalues-typeck.rs new file mode 100644 index 0000000000000..693d7f9821adb --- /dev/null +++ b/src/test/compile-fail/aggregate-rvalues-typeck.rs @@ -0,0 +1,23 @@ +// Copyright 2017 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. +//compile-flags: -Z emit-end-regions -Z borrowck-mir -Z mir + +#![allow(unused_assignments)] + +struct Wrap<'a> { w: &'a mut u32 } + +fn foo() { + let mut x = 22u64; + let wrapper = Wrap { w: &mut x }; + x += 1; //~ ERROR cannot assign to `x` + *wrapper.w += 1; +} + +fn main() { } \ No newline at end of file From fe32df9adbdc076386b5b65f74bb29fe169f1547 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 11 Nov 2017 15:32:07 -0500 Subject: [PATCH 3/9] Fix failing test --- src/test/compile-fail/aggregate-rvalues-typeck.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/test/compile-fail/aggregate-rvalues-typeck.rs b/src/test/compile-fail/aggregate-rvalues-typeck.rs index 693d7f9821adb..99f3e461ec98d 100644 --- a/src/test/compile-fail/aggregate-rvalues-typeck.rs +++ b/src/test/compile-fail/aggregate-rvalues-typeck.rs @@ -7,16 +7,22 @@ // , at your // option. This file may not be copied, modified, or distributed // except according to those terms. -//compile-flags: -Z emit-end-regions -Z borrowck-mir -Z mir +//revisions: ast mir +//[mir] compile-flags: -Z emit-end-regions -Z borrowck-mir -Z nll #![allow(unused_assignments)] struct Wrap<'a> { w: &'a mut u32 } fn foo() { - let mut x = 22u64; + let mut x = 22; let wrapper = Wrap { w: &mut x }; - x += 1; //~ ERROR cannot assign to `x` + //~^ ERROR cannot assign to `x` because it is borrowed (Mir) [E0506] + //~^^ ERROR cannot use `x` because it was mutably borrowed (Mir) [E0503] + x += 1; //[ast]~ ERROR cannot assign to `x` because it is borrowed [E0506] + //[mir]~^ ERROR cannot assign to `x` because it is borrowed (Ast) [E0506] + //[mir]~^^ ERROR cannot assign to `x` because it is borrowed (Mir) [E0506] + //[mir]~^^^ ERROR cannot use `x` because it was mutably borrowed (Mir) [E0503] *wrapper.w += 1; } From 47c6db09ee35810005274017abcd0798d6978a47 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sat, 11 Nov 2017 16:13:23 -0500 Subject: [PATCH 4/9] Remove attributes and test comments accidentally left behind, add in span_mirbugs --- src/librustc_mir/transform/type_check.rs | 29 +++++++++++++------ ...reference-carried-through-struct-field.rs} | 2 -- 2 files changed, 20 insertions(+), 11 deletions(-) rename src/test/compile-fail/{aggregate-rvalues-typeck.rs => nll/reference-carried-through-struct-field.rs} (87%) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 7a801d887fbb0..96264222eaef4 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1056,28 +1056,39 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } - #[allow(dead_code)] fn check_rvalue(&mut self, mir: &Mir<'tcx>, rv: &Rvalue<'tcx>, location: Location) { let tcx = self.tcx(); match rv { - Rvalue::Aggregate(ref ak, ref ops) => { + Rvalue::Aggregate(ak, ops) => { match **ak { // tuple rvalue field type is always the type of the op. Nothing to check here. AggregateKind::Tuple => { }, _ => { for (i, op) in ops.iter().enumerate() { - let field_ty = if let Ok(field_ty) = self.aggregate_field_ty(ak, i, location) { - field_ty - } else { - // TODO(nashenas88) log span_mirbug terr?? - continue; + let field_ty = match self.aggregate_field_ty(ak, i, location) { + Ok(field_ty) => field_ty, + Err(FieldAccessError::OutOfRange { field_count }) => { + span_mirbug!( + self, + rv, + "accessed field #{} but variant only has {}", + i, + field_count); + continue; + }, }; let op_ty = match op { Operand::Consume(lv) => lv.ty(mir, tcx).to_ty(tcx), Operand::Constant(c) => c.ty, }; - if let Err(_terr) = self.sub_types(op_ty, field_ty, location.at_successor_within_block()) { - // TODO(nashenas88) log span_mirbug terr?? + if let Err(terr) = self.sub_types(op_ty, field_ty, location.at_successor_within_block()) { + span_mirbug!( + self, + rv, + "{:?} is not a subtype of {:?}: {:?}", + op_ty, + field_ty, + terr); } } }, diff --git a/src/test/compile-fail/aggregate-rvalues-typeck.rs b/src/test/compile-fail/nll/reference-carried-through-struct-field.rs similarity index 87% rename from src/test/compile-fail/aggregate-rvalues-typeck.rs rename to src/test/compile-fail/nll/reference-carried-through-struct-field.rs index 99f3e461ec98d..afde254082946 100644 --- a/src/test/compile-fail/aggregate-rvalues-typeck.rs +++ b/src/test/compile-fail/nll/reference-carried-through-struct-field.rs @@ -17,8 +17,6 @@ struct Wrap<'a> { w: &'a mut u32 } fn foo() { let mut x = 22; let wrapper = Wrap { w: &mut x }; - //~^ ERROR cannot assign to `x` because it is borrowed (Mir) [E0506] - //~^^ ERROR cannot use `x` because it was mutably borrowed (Mir) [E0503] x += 1; //[ast]~ ERROR cannot assign to `x` because it is borrowed [E0506] //[mir]~^ ERROR cannot assign to `x` because it is borrowed (Ast) [E0506] //[mir]~^^ ERROR cannot assign to `x` because it is borrowed (Mir) [E0506] From 527a5dd2517f7ca7857ebd006a3cc6bcd0abec39 Mon Sep 17 00:00:00 2001 From: Paul Daniel Faria Date: Sun, 12 Nov 2017 00:41:29 -0500 Subject: [PATCH 5/9] Normalize LvalueTy for ops and format code to satisfy tidy check --- src/librustc_mir/transform/type_check.rs | 63 +++++++++++++++--------- 1 file changed, 39 insertions(+), 24 deletions(-) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 96264222eaef4..6888225de757c 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1012,23 +1012,27 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } - fn aggregate_field_ty(&mut self, ak: &Box>, field: usize, location: Location) - -> Result, FieldAccessError> - { + fn aggregate_field_ty( + &mut self, + ak: &Box>, + field: usize, + location: Location, + ) -> Result, FieldAccessError> { let tcx = self.tcx(); let (variant, substs) = match **ak { - AggregateKind::Adt(def, variant, substs, _) => { // handle unions? + AggregateKind::Adt(def, variant, substs, _) => { + // handle unions? (&def.variants[variant], substs) - }, + } AggregateKind::Closure(def_id, substs) => { return match substs.upvar_tys(def_id, tcx).nth(field) { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { - field_count: substs.upvar_tys(def_id, tcx).count() + field_count: substs.upvar_tys(def_id, tcx).count(), }), } - }, + } AggregateKind::Generator(def_id, substs, _) => { if let Some(ty) = substs.upvar_tys(def_id, tcx).nth(field) { return Ok(ty); @@ -1037,22 +1041,24 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { return match substs.field_tys(def_id, tcx).nth(field) { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { - field_count: substs.field_tys(def_id, tcx).count() + 1 + field_count: substs.field_tys(def_id, tcx).count() + 1, }), - } - }, + }; + } AggregateKind::Array(ty) => { return Ok(ty); - }, + } AggregateKind::Tuple => { unreachable!("This should have been covered in check_rvalues"); - }, + } }; if let Some(field) = variant.fields.get(field) { Ok(self.normalize(&field.ty(tcx, substs), location)) } else { - Err(FieldAccessError::OutOfRange { field_count: variant.fields.len() }) + Err(FieldAccessError::OutOfRange { + field_count: variant.fields.len(), + }) } } @@ -1062,7 +1068,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { Rvalue::Aggregate(ak, ops) => { match **ak { // tuple rvalue field type is always the type of the op. Nothing to check here. - AggregateKind::Tuple => { }, + AggregateKind::Tuple => {} _ => { for (i, op) in ops.iter().enumerate() { let field_ty = match self.aggregate_field_ty(ak, i, location) { @@ -1073,27 +1079,36 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { rv, "accessed field #{} but variant only has {}", i, - field_count); + field_count + ); continue; - }, + } }; let op_ty = match op { - Operand::Consume(lv) => lv.ty(mir, tcx).to_ty(tcx), + Operand::Consume(lv) => { + self.normalize(&lv.ty(mir, tcx), location).to_ty(tcx) + } Operand::Constant(c) => c.ty, }; - if let Err(terr) = self.sub_types(op_ty, field_ty, location.at_successor_within_block()) { - span_mirbug!( + if let Err(terr) = self.sub_types( + op_ty, + field_ty, + location.at_successor_within_block(), + ) + { + span_mirbug!( self, rv, "{:?} is not a subtype of {:?}: {:?}", op_ty, field_ty, - terr); - } + terr + ); + } } - }, + } } - }, + } // FIXME: These other cases have to be implemented in future PRs Rvalue::Use(..) | Rvalue::Repeat(..) | @@ -1104,7 +1119,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { Rvalue::CheckedBinaryOp(..) | Rvalue::UnaryOp(..) | Rvalue::Discriminant(..) | - Rvalue::NullaryOp(..) => { } + Rvalue::NullaryOp(..) => {} } } From b588edc41374c217b290dad0a27d6206ef183cdb Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 12 Nov 2017 06:02:29 -0500 Subject: [PATCH 6/9] only normalize operand types when in an ADT constructor --- src/librustc_mir/transform/nll/mod.rs | 2 +- src/librustc_mir/transform/type_check.rs | 27 +++++++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/librustc_mir/transform/nll/mod.rs b/src/librustc_mir/transform/nll/mod.rs index 147f061ad113f..63cbc637547d6 100644 --- a/src/librustc_mir/transform/nll/mod.rs +++ b/src/librustc_mir/transform/nll/mod.rs @@ -47,7 +47,7 @@ pub fn compute_regions<'a, 'gcx, 'tcx>( // Run the MIR type-checker. let mir_node_id = infcx.tcx.hir.as_local_node_id(def_id).unwrap(); - let constraint_sets = &type_check::type_check(infcx, mir_node_id, param_env, mir); + let constraint_sets = &type_check::type_check(infcx, mir_node_id, param_env, mir, def_id); // Create the region inference context, taking ownership of the region inference // data that was contained in `infcx`. diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 6888225de757c..cdfa506a3c2e3 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -11,6 +11,8 @@ //! This pass type-checks the MIR to ensure it is not broken. #![allow(unreachable_code)] +use rustc::hir::def_id::DefId; +use rustc::hir::map::DefPathData; use rustc::infer::{InferCtxt, InferOk, InferResult, LateBoundRegionConversionTime, UnitResult}; use rustc::infer::region_constraints::RegionConstraintData; use rustc::traits::{self, FulfillmentContext}; @@ -41,8 +43,9 @@ pub fn type_check<'a, 'gcx, 'tcx>( body_id: ast::NodeId, param_env: ty::ParamEnv<'gcx>, mir: &Mir<'tcx>, + mir_def_id: DefId, ) -> MirTypeckRegionConstraints<'tcx> { - let mut checker = TypeChecker::new(infcx, body_id, param_env); + let mut checker = TypeChecker::new(infcx, body_id, param_env, mir_def_id); let errors_reported = { let mut verifier = TypeVerifier::new(&mut checker, mir); verifier.visit_mir(mir); @@ -408,6 +411,11 @@ pub struct TypeChecker<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { body_id: ast::NodeId, reported_errors: FxHashSet<(Ty<'tcx>, Span)>, constraints: MirTypeckRegionConstraints<'tcx>, + + // FIXME(#45940) - True if this is a MIR shim or ADT constructor + // (e.g., for a tuple struct.) In that case, the internal types of + // operands and things require normalization. + is_adt_constructor: bool, } /// A collection of region constraints that must be satisfied for the @@ -459,7 +467,14 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, body_id: ast::NodeId, param_env: ty::ParamEnv<'gcx>, + mir_def_id: DefId, ) -> Self { + let def_key = infcx.tcx.def_key(mir_def_id); + let is_adt_constructor = match def_key.disambiguated_data.data { + DefPathData::StructCtor => true, + _ => false, + }; + TypeChecker { infcx, last_span: DUMMY_SP, @@ -467,6 +482,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { param_env, reported_errors: FxHashSet(), constraints: MirTypeckRegionConstraints::default(), + is_adt_constructor, } } @@ -1086,7 +1102,12 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { }; let op_ty = match op { Operand::Consume(lv) => { - self.normalize(&lv.ty(mir, tcx), location).to_ty(tcx) + let lv_ty = lv.ty(mir, tcx).to_ty(tcx); + if self.is_adt_constructor { + self.normalize(&lv_ty, location) + } else { + lv_ty + } } Operand::Constant(c) => c.ty, }; @@ -1178,7 +1199,7 @@ impl MirPass for TypeckMir { } let param_env = tcx.param_env(def_id); tcx.infer_ctxt().enter(|infcx| { - let _region_constraint_sets = type_check(&infcx, id, param_env, mir); + let _region_constraint_sets = type_check(&infcx, id, param_env, mir, def_id); // For verification purposes, we just ignore the resulting // region constraint sets. Not our problem. =) From b3a10db03ea1a0e43f58319d2d0d006bd31ded08 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 12 Nov 2017 06:25:09 -0500 Subject: [PATCH 7/9] avoid early return --- src/librustc_mir/transform/type_check.rs | 39 +++++++++++------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index cdfa506a3c2e3..464dae6d5011a 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1036,13 +1036,18 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { ) -> Result, FieldAccessError> { let tcx = self.tcx(); - let (variant, substs) = match **ak { + match **ak { AggregateKind::Adt(def, variant, substs, _) => { - // handle unions? - (&def.variants[variant], substs) + if let Some(field) = def.variants[variant].fields.get(field) { + Ok(self.normalize(&field.ty(tcx, substs), location)) + } else { + Err(FieldAccessError::OutOfRange { + field_count: variant.fields.len(), + }) + } } AggregateKind::Closure(def_id, substs) => { - return match substs.upvar_tys(def_id, tcx).nth(field) { + match substs.upvar_tys(def_id, tcx).nth(field) { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { field_count: substs.upvar_tys(def_id, tcx).count(), @@ -1051,30 +1056,22 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } AggregateKind::Generator(def_id, substs, _) => { if let Some(ty) = substs.upvar_tys(def_id, tcx).nth(field) { - return Ok(ty); + Ok(ty); + } else { + match substs.field_tys(def_id, tcx).nth(field) { + Some(ty) => Ok(ty), + None => Err(FieldAccessError::OutOfRange { + field_count: substs.field_tys(def_id, tcx).count() + 1, + }), + } } - - return match substs.field_tys(def_id, tcx).nth(field) { - Some(ty) => Ok(ty), - None => Err(FieldAccessError::OutOfRange { - field_count: substs.field_tys(def_id, tcx).count() + 1, - }), - }; } AggregateKind::Array(ty) => { - return Ok(ty); + Ok(ty) } AggregateKind::Tuple => { unreachable!("This should have been covered in check_rvalues"); } - }; - - if let Some(field) = variant.fields.get(field) { - Ok(self.normalize(&field.ty(tcx, substs), location)) - } else { - Err(FieldAccessError::OutOfRange { - field_count: variant.fields.len(), - }) } } From 10b8faccd0baeda52c0767e01d12c08440830805 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 12 Nov 2017 07:03:18 -0500 Subject: [PATCH 8/9] handle the active field index in unions --- src/librustc/mir/mod.rs | 10 +++++++--- src/librustc_mir/transform/type_check.rs | 16 +++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 355fb570c000b..94e8033ed163e 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1375,10 +1375,14 @@ pub enum AggregateKind<'tcx> { /// The type is of the element Array(Ty<'tcx>), Tuple, - /// The second field is variant number (discriminant), it's equal to 0 - /// for struct and union expressions. The fourth field is active field - /// number and is present only for union expressions. + + /// The second field is variant number (discriminant), it's equal + /// to 0 for struct and union expressions. The fourth field is + /// active field number and is present only for union expressions + /// -- e.g. for a union expression `SomeUnion { c: .. }`, the + /// active field index would identity the field `c` Adt(&'tcx AdtDef, usize, &'tcx Substs<'tcx>, Option), + Closure(DefId, ClosureSubsts<'tcx>), Generator(DefId, ClosureSubsts<'tcx>, GeneratorInterior<'tcx>), } diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index 464dae6d5011a..ebf894af180de 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -1031,14 +1031,16 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { fn aggregate_field_ty( &mut self, ak: &Box>, - field: usize, + field_index: usize, location: Location, ) -> Result, FieldAccessError> { let tcx = self.tcx(); match **ak { - AggregateKind::Adt(def, variant, substs, _) => { - if let Some(field) = def.variants[variant].fields.get(field) { + AggregateKind::Adt(def, variant_index, substs, active_field_index) => { + let variant = &def.variants[variant_index]; + let adj_field_index = active_field_index.unwrap_or(field_index); + if let Some(field) = variant.fields.get(adj_field_index) { Ok(self.normalize(&field.ty(tcx, substs), location)) } else { Err(FieldAccessError::OutOfRange { @@ -1047,7 +1049,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } AggregateKind::Closure(def_id, substs) => { - match substs.upvar_tys(def_id, tcx).nth(field) { + match substs.upvar_tys(def_id, tcx).nth(field_index) { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { field_count: substs.upvar_tys(def_id, tcx).count(), @@ -1055,10 +1057,10 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { } } AggregateKind::Generator(def_id, substs, _) => { - if let Some(ty) = substs.upvar_tys(def_id, tcx).nth(field) { - Ok(ty); + if let Some(ty) = substs.upvar_tys(def_id, tcx).nth(field_index) { + Ok(ty) } else { - match substs.field_tys(def_id, tcx).nth(field) { + match substs.field_tys(def_id, tcx).nth(field_index) { Some(ty) => Ok(ty), None => Err(FieldAccessError::OutOfRange { field_count: substs.field_tys(def_id, tcx).count() + 1, From c52e51dfb7a3d2b9027a36ba93ec73a5c1b7f00a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 20 Nov 2017 13:08:52 -0500 Subject: [PATCH 9/9] normalize types in ADT constructor Fixes #45940 --- src/librustc_mir/shim.rs | 10 ++++++-- src/librustc_mir/transform/nll/mod.rs | 2 +- src/librustc_mir/transform/type_check.rs | 32 +++--------------------- 3 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index d31f3812e9a1d..15c68954230ba 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -825,10 +825,16 @@ pub fn build_adt_ctor<'a, 'gcx, 'tcx>(infcx: &infer::InferCtxt<'a, 'gcx, 'tcx>, -> Mir<'tcx> { let tcx = infcx.tcx; + let gcx = tcx.global_tcx(); let def_id = tcx.hir.local_def_id(ctor_id); - let sig = tcx.no_late_bound_regions(&tcx.fn_sig(def_id)) + let sig = gcx.no_late_bound_regions(&gcx.fn_sig(def_id)) .expect("LBR in ADT constructor signature"); - let sig = tcx.erase_regions(&sig); + let sig = gcx.erase_regions(&sig); + let param_env = gcx.param_env(def_id); + + // Normalize the sig now that we have liberated the late-bound + // regions. + let sig = gcx.normalize_associated_type_in_env(&sig, param_env); let (adt_def, substs) = match sig.output().sty { ty::TyAdt(adt_def, substs) => (adt_def, substs), diff --git a/src/librustc_mir/transform/nll/mod.rs b/src/librustc_mir/transform/nll/mod.rs index 63cbc637547d6..147f061ad113f 100644 --- a/src/librustc_mir/transform/nll/mod.rs +++ b/src/librustc_mir/transform/nll/mod.rs @@ -47,7 +47,7 @@ pub fn compute_regions<'a, 'gcx, 'tcx>( // Run the MIR type-checker. let mir_node_id = infcx.tcx.hir.as_local_node_id(def_id).unwrap(); - let constraint_sets = &type_check::type_check(infcx, mir_node_id, param_env, mir, def_id); + let constraint_sets = &type_check::type_check(infcx, mir_node_id, param_env, mir); // Create the region inference context, taking ownership of the region inference // data that was contained in `infcx`. diff --git a/src/librustc_mir/transform/type_check.rs b/src/librustc_mir/transform/type_check.rs index ebf894af180de..ade5cf8b70c2b 100644 --- a/src/librustc_mir/transform/type_check.rs +++ b/src/librustc_mir/transform/type_check.rs @@ -11,8 +11,6 @@ //! This pass type-checks the MIR to ensure it is not broken. #![allow(unreachable_code)] -use rustc::hir::def_id::DefId; -use rustc::hir::map::DefPathData; use rustc::infer::{InferCtxt, InferOk, InferResult, LateBoundRegionConversionTime, UnitResult}; use rustc::infer::region_constraints::RegionConstraintData; use rustc::traits::{self, FulfillmentContext}; @@ -43,9 +41,8 @@ pub fn type_check<'a, 'gcx, 'tcx>( body_id: ast::NodeId, param_env: ty::ParamEnv<'gcx>, mir: &Mir<'tcx>, - mir_def_id: DefId, ) -> MirTypeckRegionConstraints<'tcx> { - let mut checker = TypeChecker::new(infcx, body_id, param_env, mir_def_id); + let mut checker = TypeChecker::new(infcx, body_id, param_env); let errors_reported = { let mut verifier = TypeVerifier::new(&mut checker, mir); verifier.visit_mir(mir); @@ -411,11 +408,6 @@ pub struct TypeChecker<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { body_id: ast::NodeId, reported_errors: FxHashSet<(Ty<'tcx>, Span)>, constraints: MirTypeckRegionConstraints<'tcx>, - - // FIXME(#45940) - True if this is a MIR shim or ADT constructor - // (e.g., for a tuple struct.) In that case, the internal types of - // operands and things require normalization. - is_adt_constructor: bool, } /// A collection of region constraints that must be satisfied for the @@ -467,14 +459,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, body_id: ast::NodeId, param_env: ty::ParamEnv<'gcx>, - mir_def_id: DefId, ) -> Self { - let def_key = infcx.tcx.def_key(mir_def_id); - let is_adt_constructor = match def_key.disambiguated_data.data { - DefPathData::StructCtor => true, - _ => false, - }; - TypeChecker { infcx, last_span: DUMMY_SP, @@ -482,7 +467,6 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { param_env, reported_errors: FxHashSet(), constraints: MirTypeckRegionConstraints::default(), - is_adt_constructor, } } @@ -1099,17 +1083,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { continue; } }; - let op_ty = match op { - Operand::Consume(lv) => { - let lv_ty = lv.ty(mir, tcx).to_ty(tcx); - if self.is_adt_constructor { - self.normalize(&lv_ty, location) - } else { - lv_ty - } - } - Operand::Constant(c) => c.ty, - }; + let op_ty = op.ty(mir, tcx); if let Err(terr) = self.sub_types( op_ty, field_ty, @@ -1198,7 +1172,7 @@ impl MirPass for TypeckMir { } let param_env = tcx.param_env(def_id); tcx.infer_ctxt().enter(|infcx| { - let _region_constraint_sets = type_check(&infcx, id, param_env, mir, def_id); + let _region_constraint_sets = type_check(&infcx, id, param_env, mir); // For verification purposes, we just ignore the resulting // region constraint sets. Not our problem. =)