From 6db4beb3e833eb7d8daad30e2a9ecc27d54a0318 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Fri, 12 May 2017 05:43:48 -0400 Subject: [PATCH] use equality in the coerce-unsized check This seems both to be a safe, conservative choice, and it sidesteps the cycle in #41936. Fixes #41936. --- src/librustc/infer/combine.rs | 19 +++++++ src/librustc_typeck/coherence/builtin.rs | 51 ++++++++++++++++++- ...sue-41936-variance-coerce-unsized-cycle.rs | 38 ++++++++++++++ 3 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/issue-41936-variance-coerce-unsized-cycle.rs diff --git a/src/librustc/infer/combine.rs b/src/librustc/infer/combine.rs index 18909a784d610..aabb6aff55140 100644 --- a/src/librustc/infer/combine.rs +++ b/src/librustc/infer/combine.rs @@ -39,10 +39,12 @@ use super::sub::Sub; use super::InferCtxt; use super::{MiscVariable, TypeTrace}; +use hir::def_id::DefId; use ty::{IntType, UintType}; use ty::{self, Ty, TyCtxt}; use ty::error::TypeError; use ty::relate::{self, Relate, RelateResult, TypeRelation}; +use ty::subst::Substs; use traits::{Obligation, PredicateObligations}; use syntax::ast; @@ -336,6 +338,23 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, ' Ok(ty::Binder(self.relate(a.skip_binder(), b.skip_binder())?)) } + fn relate_item_substs(&mut self, + item_def_id: DefId, + a_subst: &'tcx Substs<'tcx>, + b_subst: &'tcx Substs<'tcx>) + -> RelateResult<'tcx, &'tcx Substs<'tcx>> + { + if self.ambient_variance == ty::Variance::Invariant { + // Avoid fetching the variance if we are in an invariant + // context; no need, and it can induce dependency cycles + // (e.g. #41849). + relate::relate_substs(self, None, a_subst, b_subst) + } else { + let opt_variances = self.tcx().variances_of(item_def_id); + relate::relate_substs(self, Some(&opt_variances), a_subst, b_subst) + } + } + fn relate_with_variance>(&mut self, variance: ty::Variance, a: &T, diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index d40a68e605690..556bd618c78cb 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -249,6 +249,45 @@ pub fn coerce_unsized_info<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, return err_info; } + // Here we are considering a case of converting + // `S` to S`. As an example, let's imagine a struct `Foo`, + // which acts like a pointer to `U`, but carries along some extra data of type `T`: + // + // struct Foo { + // extra: T, + // ptr: *mut U, + // } + // + // We might have an impl that allows (e.g.) `Foo` to be unsized + // to `Foo`. That impl would look like: + // + // impl, V> CoerceUnsized> for Foo {} + // + // Here `U = [i32; 3]` and `V = [i32]`. At runtime, + // when this coercion occurs, we would be changing the + // field `ptr` from a thin pointer of type `*mut [i32; + // 3]` to a fat pointer of type `*mut [i32]` (with + // extra data `3`). **The purpose of this check is to + // make sure that we know how to do this conversion.** + // + // To check if this impl is legal, we would walk down + // the fields of `Foo` and consider their types with + // both substitutes. We are looking to find that + // exactly one (non-phantom) field has changed its + // type, which we will expect to be the pointer that + // is becoming fat (we could probably generalize this + // to mutiple thin pointers of the same type becoming + // fat, but we don't). In this case: + // + // - `extra` has type `T` before and type `T` after + // - `ptr` has type `*mut U` before and type `*mut V` after + // + // Since just one field changed, we would then check + // that `*mut U: CoerceUnsized<*mut V>` is implemented + // (in other words, that we know how to do this + // conversion). This will work out because `U: + // Unsize`, and we have a builtin rule that `*mut + // U` can be coerced to `*mut V` if `U: Unsize`. let fields = &def_a.struct_variant().fields; let diff_fields = fields.iter() .enumerate() @@ -260,8 +299,16 @@ pub fn coerce_unsized_info<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, return None; } - // Ignore fields that aren't significantly changed - if let Ok(ok) = infcx.sub_types(false, &cause, b, a) { + // Ignore fields that aren't changed; it may + // be that we could get away with subtyping or + // something more accepting, but we use + // equality because we want to be able to + // perform this check without computing + // variance where possible. (This is because + // we may have to evaluate constraint + // expressions in the course of execution.) + // See e.g. #41936. + if let Ok(ok) = infcx.eq_types(false, &cause, b, a) { if ok.obligations.is_empty() { return None; } diff --git a/src/test/run-pass/issue-41936-variance-coerce-unsized-cycle.rs b/src/test/run-pass/issue-41936-variance-coerce-unsized-cycle.rs new file mode 100644 index 0000000000000..bfbead8789178 --- /dev/null +++ b/src/test/run-pass/issue-41936-variance-coerce-unsized-cycle.rs @@ -0,0 +1,38 @@ +// Copyright 2016 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. + +// Regression test for #41936. The coerce-unsized trait check in +// coherence was using subtyping, which triggered variance +// computation, which failed because it required type info for fields +// that had not (yet) been computed. + +#![feature(unsize)] +#![feature(coerce_unsized)] + +use std::{marker,ops}; + +// Change the array to a non-array, and error disappears +// Adding a new field to the end keeps the error +struct LogDataBuf([u8;8]); + +struct Aref +{ + // Inner structure triggers the error, removing the inner removes the message. + ptr: Box>, +} +impl, U: ?Sized> ops::CoerceUnsized> for Aref {} + +struct ArefInner +{ + // Even with this field commented out, the error is raised. + data: T, +} + +fn main(){}