mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Address comments from @pnkfelix (thanks for the detailed review)

Browse Source
This commit is contained in:
Niko Matsakis 2013-11-08 15:52:36 -05:00
parent bc3e842508
commit f36a891fe2
11 changed files with 282 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/librustc/middle/ty.rs
View File

@ -218,10 +218,10 @@ pub struct ItemVariances {
#[deriving(Clone, Eq, Decodable, Encodable)]
pub enum Variance {
Covariant,
Invariant,
Contravariant,
Bivariant,
Covariant, // T<A> <: T<B> iff A <: B -- e.g., function return type
Invariant, // T<A> <: T<B> iff B == A -- e.g., type of mutable cell
Contravariant, // T<A> <: T<B> iff B <: A -- e.g., function param type
Bivariant, // T<A> <: T<B> -- e.g., unused type parameter
}
#[deriving(Decodable, Encodable)]

17
src/librustc/middle/typeck/astconv.rs
View File

@ -116,7 +116,7 @@ pub fn ast_region_to_region(
r
}
pub fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
this: &AC,
rscope: &RS,
default_span: Span,
@ -129,14 +129,14 @@ pub fn opt_ast_region_to_region<AC:AstConv,RS:RegionScope>(
None => {
match rscope.anon_regions(default_span, 1) {
None => {
Err(()) => {
debug!("optional region in illegal location");
this.tcx().sess.span_err(
default_span, "missing lifetime specifier");
ty::ReStatic
}
Some(rs) => {
Ok(rs) => {
rs[0]
}
}
@ -178,7 +178,7 @@ fn ast_path_substs<AC:AstConv,RS:RegionScope>(
let anon_regions =
rscope.anon_regions(path.span, expected_num_region_params);
if supplied_num_region_params != 0 || anon_regions.is_none() {
if supplied_num_region_params != 0 || anon_regions.is_err() {
tcx.sess.span_err(
path.span,
format!("wrong number of lifetime parameters: \
@ -188,9 +188,9 @@ fn ast_path_substs<AC:AstConv,RS:RegionScope>(
}
match anon_regions {
Some(v) => opt_vec::from(v),
None => opt_vec::from(vec::from_fn(expected_num_region_params,
|_| ty::ReStatic)) // hokey
Ok(v) => opt_vec::from(v),
Err(()) => opt_vec::from(vec::from_fn(expected_num_region_params,
|_| ty::ReStatic)) // hokey
}
};
@ -277,8 +277,7 @@ pub static NO_REGIONS: uint = 1;
pub static NO_TPS: uint = 2;
// Parses the programmer's textual representation of a type into our
// internal notion of a type. `getter` is a function that returns the type
// corresponding to a definition ID:
// internal notion of a type.
pub fn ast_ty_to_ty<AC:AstConv, RS:RegionScope>(
this: &AC, rscope: &RS, ast_ty: &ast::Ty) -> ty::t {

5
src/librustc/middle/typeck/check/mod.rs
View File

@ -800,7 +800,6 @@ fn check_impl_methods_against_trait(ccx: @mut CrateCtxt,
* - impl_m_body_id: id of the method body
* - trait_m: the method in the trait
* - trait_substs: the substitutions used on the type of the trait
* - self_ty: the self type of the impl
*/
pub fn compare_impl_method(tcx: ty::ctxt,
impl_generics: &ty::Generics,
@ -1062,8 +1061,8 @@ impl FnCtxt {
impl RegionScope for @mut infer::InferCtxt {
fn anon_regions(&self,
span: Span,
count: uint) -> Option<~[ty::Region]> {
Some(vec::from_fn(
count: uint) -> Result<~[ty::Region], ()> {
Ok(vec::from_fn(
count,
|_| self.next_region_var(infer::MiscVariable(span))))
}

22
src/librustc/middle/typeck/collect.rs
View File

@ -517,17 +517,17 @@ pub fn convert(ccx: &CrateCtxt, it: &ast::item) {
let tcx = ccx.tcx;
debug!("convert: item {} with id {}", tcx.sess.str_of(it.ident), it.id);
match it.node {
// These don't define types.
ast::item_foreign_mod(_) | ast::item_mod(_) => {}
ast::item_enum(ref enum_definition, ref generics) => {
ensure_no_ty_param_bounds(ccx, it.span, generics, "enumeration");
let tpt = ty_of_item(ccx, it);
write_ty_to_tcx(tcx, it.id, tpt.ty);
get_enum_variant_types(ccx,
tpt.ty,
enum_definition.variants,
generics);
}
// These don't define types.
ast::item_foreign_mod(_) | ast::item_mod(_) => {}
ast::item_enum(ref enum_definition, ref generics) => {
ensure_no_ty_param_bounds(ccx, it.span, generics, "enumeration");
let tpt = ty_of_item(ccx, it);
write_ty_to_tcx(tcx, it.id, tpt.ty);
get_enum_variant_types(ccx,
tpt.ty,
enum_definition.variants,
generics);
}
ast::item_impl(ref generics, ref opt_trait_ref, ref selfty, ref ms) => {
let i_ty_generics = ty_generics(ccx, generics, 0);
let selfty = ccx.to_ty(&ExplicitRscope, selfty);

24
src/librustc/middle/typeck/rscope.rs
View File

@ -16,11 +16,21 @@ use syntax::ast;
use syntax::codemap::Span;
use syntax::opt_vec::OptVec;
/// Defines strategies for handling regions that are omitted. For
/// example, if one writes the type `&Foo`, then the lifetime of of
/// this borrowed pointer has been omitted. When converting this
/// type, the generic functions in astconv will invoke `anon_regions`
/// on the provided region-scope to decide how to translate this
/// omitted region.
///
/// It is not always legal to omit regions, therefore `anon_regions`
/// can return `Err(())` to indicate that this is not a scope in which
/// regions can legally be omitted.
pub trait RegionScope {
fn anon_regions(&self,
span: Span,
count: uint)
-> Option<~[ty::Region]>;
-> Result<~[ty::Region], ()>;
}
// A scope in which all regions must be explicitly named
@ -30,11 +40,13 @@ impl RegionScope for ExplicitRscope {
fn anon_regions(&self,
_span: Span,
_count: uint)
-> Option<~[ty::Region]> {
None
-> Result<~[ty::Region], ()> {
Err(())
}
}
/// A scope in which we generate anonymous, late-bound regions for
/// omitted regions. This occurs in function signatures.
pub struct BindingRscope {
binder_id: ast::NodeId,
anon_bindings: @mut uint
@ -53,11 +65,11 @@ impl RegionScope for BindingRscope {
fn anon_regions(&self,
_: Span,
count: uint)
-> Option<~[ty::Region]> {
-> Result<~[ty::Region], ()> {
let idx = *self.anon_bindings;
*self.anon_bindings += count;
Some(vec::from_fn(count, |i| ty::ReLateBound(self.binder_id,
ty::BrAnon(idx + i))))
Ok(vec::from_fn(count, |i| ty::ReLateBound(self.binder_id,
ty::BrAnon(idx + i))))
}
}

193
src/librustc/middle/typeck/variance.rs
View File

@ -1,4 +1,4 @@
// Copyright 2012 The Rust Project Developers. See the COPYRIGHT
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
@ -15,13 +15,137 @@ algorithm is taken from Section 4 of the paper "Taming the Wildcards:
Combining Definition- and Use-Site Variance" published in PLDI'11 and
written by Altidor et al., and hereafter referred to as The Paper.
This inference is explicitly designed *not* to consider the uses of
types within code. To determine the variance of type parameters
defined on type `X`, we only consider the definition of the type `X`
and the definitions of any types it references.
We only infer variance for type parameters found on *types*: structs,
enums, and traits. We do not infer variance for type parameters found
on fns or impls. This is because those things are not type definitions
and variance doesn't really make sense in that context.
It is worth covering what variance means in each case. For structs and
enums, I think it is fairly straightforward. The variance of the type
or lifetime parameters defines whether `T<A>` is a subtype of `T<B>`
(resp. `T<'a>` and `T<'b>`) based on the relationship of `A` and `B`
(resp. `'a` and `'b`). (FIXME #3598 -- we do not currently make use of
the variances we compute for type parameters.)
### Variance on traits
The meaning of variance for trait parameters is more subtle and worth
expanding upon. There are in fact two uses of the variance values we
compute.
#### Trait variance and object types
The first is for object types. Just as with structs and enums, we can
decide the subtyping relationship between two object types `&Trait<A>`
and `&Trait<B>` based on the relationship of `A` and `B`. Note that
for object types we ignore the `Self` type parameter -- it is unknown,
and the nature of dynamic dispatch ensures that we will always call a
function that is expected the appropriate `Self` type. However, we
must be careful with the other type parameters, or else we could end
up calling a function that is expecting one type but provided another.
To see what I mean, consider a trait like so:
trait ConvertTo<A> {
fn convertTo(&self) -> A;
}
Intuitively, If we had one object `O=&ConvertTo<Object>` and another
`S=&ConvertTo<String>`, then `S <: O` because `String <: Object`
(presuming Java-like "string" and "object" types, my go to examples
for subtyping). The actual algorithm would be to compare the
(explicit) type parameters pairwise respecting their variance: here,
the type parameter A is covariant (it appears only in a return
position), and hence we require that `String <: Object`.
You'll note though that we did not consider the binding for the
(implicit) `Self` type parameter: in fact, it is unknown, so that's
good. The reason we can ignore that parameter is precisely because we
don't need to know its value until a call occurs, and at that time (as
you said) the dynamic nature of virtual dispatch means the code we run
will be correct for whatever value `Self` happens to be bound to for
the particular object whose method we called. `Self` is thus different
from `A`, because the caller requires that `A` be known in order to
know the return type of the method `convertTo()`. (As an aside, we
have rules preventing methods where `Self` appears outside of the
receiver position from being called via an object.)
#### Trait variance and vtable resolution
But traits aren't only used with objects. They're also used when
deciding whether a given impl satisfies a given trait bound (or should
be -- FIXME #5781). To set the scene here, imagine I had a function:
fn convertAll<A,T:ConvertTo<A>>(v: &[T]) {
...
}
Now imagine that I have an implementation of `ConvertTo` for `Object`:
impl ConvertTo<int> for Object { ... }
And I want to call `convertAll` on an array of strings. Suppose
further that for whatever reason I specifically supply the value of
`String` for the type parameter `T`:
let mut vector = ~["string", ...];
convertAll::<int, String>(v);
Is this legal? To put another way, can we apply the `impl` for
`Object` to the type `String`? The answer is yes, but to see why
we have to expand out what will happen:
- `convertAll` will create a pointer to one of the entries in the
vector, which will have type `&String`
- It will then call the impl of `convertTo()` that is intended
for use with objects. This has the type:
fn(self: &Object) -> int
It is ok to provide a value for `self` of type `&String` because
`&String <: &Object`.
OK, so intuitively we want this to be legal, so let's bring this back
to variance and see whether we are computing the correct result. We
must first figure out how to phrase the question "is an impl for
`Object,int` usable where an impl for `String,int` is expected?"
Maybe it's helpful to think of a dictionary-passing implementation of
type classes. In that case, `convertAll()` takes an implicit parameter
representing the impl. In short, we *have* an impl of type:
V_O = ConvertTo<int> for Object
and the function prototype expects an impl of type:
V_S = ConvertTo<int> for String
As with any argument, this is legal if the type of the value given
(`V_O`) is a subtype of the type expected (`V_S`). So is `V_O <: V_S`?
The answer will depend on the variance of the various parameters. In
this case, because the `Self` parameter is contravariant and `A` is
covariant, it means that:
V_O <: V_S iff
int <: int
String <: Object
These conditions are satisfied and so we are happy.
### The algorithm
The basic idea is quite straightforward. We iterate over the types
defined and, for each use of a type parameter X, accumulate a
constraint indicating that the variance of X must be valid for the
variance of that use site. We then iteratively refine the variance of
X until all constraints are met. There is *always* a sol'n, because at
the limit we can declare all type parameters to be invariant and all
constriants will be satisfied.
constraints will be satisfied.
As a simple example, consider:
@ -46,8 +170,8 @@ results are based on a variance lattice defined as follows:
o Bottom (invariant)
Based on this lattice, the solution V(A)=+, V(B)=-, V(C)=o is the
minimal solution (which is what we are looking for; the maximal
solution is just that all variables are invariant. Not so exciting.).
optimal solution. Note that there is always a naive solution which
just declares all variables to be invariant.
You may be wondering why fixed-point iteration is required. The reason
is that the variance of a use site may itself be a function of the
@ -59,9 +183,12 @@ take the form:
Here the notation V(X) indicates the variance of a type/region
parameter `X` with respect to its defining class. `Term x Term`
represents the "variance transform" as defined in the paper -- `V1 x
V2` is the resulting variance when a use site with variance V2 appears
inside a use site with variance V1.
represents the "variance transform" as defined in the paper:
If the variance of a type variable `X` in type expression `E` is `V2`
and the definition-site variance of the [corresponding] type parameter
of a class `C` is `V1`, then the variance of `X` in the type expression
`C<E>` is `V3 = V1.xform(V2)`.
*/
@ -128,9 +255,13 @@ struct TermsContext<'self> {
tcx: ty::ctxt,
arena: &'self Arena,
empty_variances: @ty::ItemVariances,
// Maps from the node id of a type/generic parameter to the
// corresponding inferred index.
inferred_map: HashMap<ast::NodeId, InferredIndex>,
// Maps from an InferredIndex to the info for that variable.
inferred_infos: ~[InferredInfo<'self>],
}
@ -153,6 +284,12 @@ fn determine_parameters_to_be_inferred<'a>(tcx: ty::ctxt,
arena: arena,
inferred_map: HashMap::new(),
inferred_infos: ~[],
// cache and share the variance struct used for items with
// no type/region parameters
empty_variances: @ty::ItemVariances { self_param: None,
type_params: opt_vec::Empty,
region_params: opt_vec::Empty }
};
visit::walk_crate(&mut terms_cx, crate, ());
@ -228,11 +365,7 @@ impl<'self> Visitor<()> for TermsContext<'self> {
if self.num_inferred() == inferreds_on_entry {
let newly_added = self.tcx.item_variance_map.insert(
ast_util::local_def(item.id),
@ty::ItemVariances {
self_param: None,
type_params: opt_vec::Empty,
region_params: opt_vec::Empty
});
self.empty_variances);
assert!(newly_added);
}
@ -262,6 +395,7 @@ impl<'self> Visitor<()> for TermsContext<'self> {
struct ConstraintContext<'self> {
terms_cx: TermsContext<'self>,
// These are pointers to common `ConstantTerm` instances
covariant: VarianceTermPtr<'self>,
contravariant: VarianceTermPtr<'self>,
invariant: VarianceTermPtr<'self>,
@ -309,7 +443,7 @@ impl<'self> Visitor<()> for ConstraintContext<'self> {
// annoyingly takes it upon itself to run off and
// evaluate the discriminants eagerly (*grumpy* that's
// not the typical pattern). This results in double
// error messagees because typeck goes off and does
// error messages because typeck goes off and does
// this at a later time. All we really care about is
// the types of the variant arguments, so we just call
// `ty::VariantInfo::from_ast_variant()` ourselves
@ -340,8 +474,14 @@ impl<'self> Visitor<()> for ConstraintContext<'self> {
for method in methods.iter() {
match method.transformed_self_ty {
Some(self_ty) => {
// The self type is a parameter, so its type
// should be considered contravariant:
// The implicit self parameter is basically
// equivalent to a normal parameter declared
// like:
//
// self : self_ty
//
// where self_ty is `&Self` or `&mut Self`
// or whatever.
self.add_constraints_from_ty(
self_ty, self.contravariant);
}
@ -465,6 +605,8 @@ impl<'self> ConstraintContext<'self> {
}
}
/// Adds constraints appropriate for an instance of `ty` appearing
/// in a context with ambient variance `variance`
fn add_constraints_from_ty(&mut self,
ty: ty::t,
variance: VarianceTermPtr<'self>) {
@ -558,6 +700,8 @@ impl<'self> ConstraintContext<'self> {
}
}
/// Adds constraints appropriate for a vector with vstore `vstore`
/// appearing in a context with ambient variance `variance`
fn add_constraints_from_vstore(&mut self,
vstore: ty::vstore,
variance: VarianceTermPtr<'self>) {
@ -572,6 +716,8 @@ impl<'self> ConstraintContext<'self> {
}
}
/// Adds constraints appropriate for a nominal type (enum, struct,
/// object, etc) appearing in a context with ambient variance `variance`
fn add_constraints_from_substs(&mut self,
def_id: ast::DefId,
generics: &ty::Generics,
@ -599,6 +745,8 @@ impl<'self> ConstraintContext<'self> {
}
}
/// Adds constraints appropriate for a function with signature
/// `sig` appearing in a context with ambient variance `variance`
fn add_constraints_from_sig(&mut self,
sig: &ty::FnSig,
variance: VarianceTermPtr<'self>) {
@ -609,6 +757,8 @@ impl<'self> ConstraintContext<'self> {
self.add_constraints_from_ty(sig.output, variance);
}
/// Adds constraints appropriate for a region appearing in a
/// context with ambient variance `variance`
fn add_constraints_from_region(&mut self,
region: ty::Region,
variance: VarianceTermPtr<'self>) {
@ -636,6 +786,8 @@ impl<'self> ConstraintContext<'self> {
}
}
/// Adds constraints appropriate for a mutability-type pair
/// appearing in a context with ambient variance `variance`
fn add_constraints_from_mt(&mut self,
mt: &ty::mt,
variance: VarianceTermPtr<'self>) {
@ -657,13 +809,15 @@ impl<'self> ConstraintContext<'self> {
*
* The final phase iterates over the constraints, refining the variance
* for each inferred until a fixed point is reached. This will be the
* maximal solution to the constraints. The final variance for each
* optimal solution to the constraints. The final variance for each
* inferred is then written into the `variance_map` in the tcx.
*/
struct SolveContext<'self> {
terms_cx: TermsContext<'self>,
constraints: ~[Constraint<'self>],
// Maps from an InferredIndex to the inferred value for that variable.
solutions: ~[ty::Variance]
}
@ -715,7 +869,12 @@ impl<'self> SolveContext<'self> {
// Collect all the variances for a particular item and stick
// them into the variance map. We rely on the fact that we
// generate all the inferreds for a particular item
// consecutively.
// consecutively (that is, we collect solutions for an item
// until we see a new item id, and we assume (1) the solutions
// are in the same order as the type parameters were declared
// and (2) all solutions or a given item appear before a new
// item id).
let tcx = self.terms_cx.tcx;
let item_variance_map = tcx.item_variance_map;
let solutions = &self.solutions;

6
src/libsyntax/ast.rs
View File

@ -259,12 +259,6 @@ pub enum DefRegion {
DefFreeRegion(/* block scope */ NodeId, /* lifetime decl */ NodeId),
}
#[deriving(Clone, Eq, IterBytes, Encodable, Decodable, ToStr)]
pub struct DefNamedRegion {
node_id: NodeId,
depth: uint,
}
// The set of MetaItems that define the compilation environment of the crate,
// used to drive conditional compilation
pub type CrateConfig = ~[@MetaItem];

37
src/test/compile-fail/regions-variance-contravariant-use-covariant.rs Normal file
View File

@ -0,0 +1,37 @@
// 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 that a type which is covariant with respect to its region
// parameter yields an error when used in a contravariant way.
//
// Note: see variance-regions-*.rs for the tests that check that the
// variance inference works in the first place.
// This is covariant with respect to 'a, meaning that
// Covariant<'foo> <: Covariant<'static> because
// 'foo <= 'static
struct Contravariant<'a> {
f: &'a int
}
fn use_<'short,'long>(c: Contravariant<'short>,
s: &'short int,
l: &'long int,
_where:Option<&'short &'long ()>) {
// Test whether Contravariant<'short> <: Contravariant<'long>. Since
// 'short <= 'long, this would be true if the Contravariant type were
// covariant with respect to its parameter 'a.
let _: Contravariant<'long> = c; //~ ERROR mismatched types
//~^ ERROR cannot infer an appropriate lifetime
}
fn main() {}

23
src/test/compile-fail/regions-variance-covariant-use-contravariant.rs
View File

@ -8,8 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Test that a type which is contravariant with respect to its region
// parameter yields an error when used in a covariant way.
// Test that a type which is covariant with respect to its region
// parameter yields an error when used in a contravariant way.
//
// Note: see variance-regions-*.rs for the tests that check that the
// variance inference works in the first place.
@ -21,18 +21,17 @@ struct Covariant<'a> {
f: extern "Rust" fn(&'a int)
}
fn use_<'a>(c: Covariant<'a>) {
let x = 3;
fn use_<'short,'long>(c: Covariant<'long>,
s: &'short int,
l: &'long int,
_where:Option<&'short &'long ()>) {
// 'b winds up being inferred to 'a because
// Covariant<'a> <: Covariant<'b> => 'a <= 'b
//
// Borrow checker then reports an error because `x` does not
// have the lifetime 'a.
collapse(&x, c); //~ ERROR borrowed value does not live long enough
// Test whether Covariant<'long> <: Covariant<'short>. Since
// 'short <= 'long, this would be true if the Covariant type were
// contravariant with respect to its parameter 'a.
fn collapse<'b>(x: &'b int, c: Covariant<'b>) { }
let _: Covariant<'short> = c; //~ ERROR mismatched types
//~^ ERROR cannot infer an appropriate lifetime
}
fn main() {}

18
src/test/compile-fail/regions-variance-invariant-use-contravariant.rs
View File

@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// Test that a covariant region parameter used in a covariant position
// Test that an invariant region parameter used in a contravariant way
// yields an error.
//
// Note: see variance-regions-*.rs for the tests that check that the
@ -18,16 +18,16 @@ struct Invariant<'a> {
f: &'static mut &'a int
}
fn use_<'a>(c: Invariant<'a>) {
let x = 3;
fn use_<'short,'long>(c: Invariant<'long>,
s: &'short int,
l: &'long int,
_where:Option<&'short &'long ()>) {
// 'b winds up being inferred to 'a, because that is the
// only way that Invariant<'a> <: Invariant<'b>, and hence
// we get an error in the borrow checker because &x cannot
// live that long
collapse(&x, c); //~ ERROR borrowed value does not live long enough
// Test whether Invariant<'long> <: Invariant<'short>. Since
// 'short <= 'long, this would be true if the Invariant type were
// contravariant with respect to its parameter 'a.
fn collapse<'b>(x: &'b int, c: Invariant<'b>) { }
let _: Invariant<'short> = c; //~ ERROR lifetime mistach
}
fn main() { }

9
src/test/compile-fail/regions-variance-invariant-use-covariant.rs
View File

@ -18,9 +18,12 @@ struct Invariant<'a> {
f: &'static mut &'a int
}
fn use_<'a>(c: Invariant<'a>) {
// For this assignment to be legal, Invariant<'a> <: Invariant<'static>,
// which (if Invariant were covariant) would require 'a <= 'static.
fn use_<'b>(c: Invariant<'b>) {
// For this assignment to be legal, Invariant<'b> <: Invariant<'static>.
// Since 'b <= 'static, this would be true if Invariant were covariant
// with respect to its parameter 'a.
let _: Invariant<'static> = c; //~ ERROR mismatched types
}