Skip to content

Commit

Permalink
Auto merge of #46054 - nikomatsakis:nll-master-to-rust-master-1, r=ar…
Browse files Browse the repository at this point in the history
…ielb1

typeck aggregate rvalues in MIR type checker

This branch is an attempt to land content by @spastorino and @Nashenas88 that was initially landed on nll-master while we waited for #45825 to land.

The biggest change it contains is that it extends the MIR type-checker to also type-check MIR aggregate rvalues (at least partially). Specifically, it checks that the operands provided for each field have the right type.

It does not yet check that their well-formedness predicates are met. That is #45827. It also does not check other kinds of rvalues (that is #45959). @spastorino is working on those issues now.

r? @arielb1
  • Loading branch information
bors committed Nov 23, 2017
2 parents 247d98e + c52e51d commit b9b82fd
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 37 deletions.
18 changes: 9 additions & 9 deletions src/librustc/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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) => {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/infer/unify_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<ty::RegionVid>) -> &'static str { "RegionVid" }
}

Expand Down
10 changes: 7 additions & 3 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>),

Closure(DefId, ClosureSubsts<'tcx>),
Generator(DefId, ClosureSubsts<'tcx>, GeneratorInterior<'tcx>),
}
Expand Down
21 changes: 5 additions & 16 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_) |
Expand Down Expand Up @@ -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())
}
}

Expand Down
10 changes: 8 additions & 2 deletions src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
106 changes: 106 additions & 0 deletions src/librustc_mir/transform/type_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
terr
);
}
self.check_rvalue(mir, rv, location);
}
StatementKind::SetDiscriminant {
ref lvalue,
Expand Down Expand Up @@ -1011,6 +1012,111 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> {
}
}

fn aggregate_field_ty(
&mut self,
ak: &Box<AggregateKind<'tcx>>,
field_index: usize,
location: Location,
) -> Result<Ty<'tcx>, FieldAccessError> {
let tcx = self.tcx();

match **ak {
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 {
field_count: variant.fields.len(),
})
}
}
AggregateKind::Closure(def_id, substs) => {
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(),
}),
}
}
AggregateKind::Generator(def_id, substs, _) => {
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_index) {
Some(ty) => Ok(ty),
None => Err(FieldAccessError::OutOfRange {
field_count: substs.field_tys(def_id, tcx).count() + 1,
}),
}
}
}
AggregateKind::Array(ty) => {
Ok(ty)
}
AggregateKind::Tuple => {
unreachable!("This should have been covered in check_rvalues");
}
}
}

fn check_rvalue(&mut self, mir: &Mir<'tcx>, rv: &Rvalue<'tcx>, location: Location) {
let tcx = self.tcx();
match rv {
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 = 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 = op.ty(mir, tcx);
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
);
}
}
}
}
}
// 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// 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 <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.
//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 = 22;
let wrapper = Wrap { w: &mut x };
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;
}

fn main() { }

0 comments on commit b9b82fd

Please sign in to comment.