Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infer or permit declarations of variance for type parameters #3598

Closed
nikomatsakis opened this issue Sep 26, 2012 · 26 comments
Closed

infer or permit declarations of variance for type parameters #3598

nikomatsakis opened this issue Sep 26, 2012 · 26 comments
Labels
A-type-system Area: Type system E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-medium Medium priority

Comments

@nikomatsakis
Copy link
Contributor

UPDATE

Variance inference is complete.

What is still missing is:

ORIGINAL

Today, we treat all type parameters as invariant, but infer variance for region parameters. We can use that same code which infers the variance for region parameters to infer the suitable variance for type parameters if we like.

Alternatively, if we decide that inference here is too magical, we could require explicit declaration of variance. In that case, i'd favor covariance as the default, and the possibility to write struct Foo<mut T> to indicate an invariant type parameter T (which can appear in mutable locations). This seems more intuitive than the usual + and -. We probably don't need a notation for contravariance, but in that case perhaps fn T (to indicate a type parameter that appears only in function arguments, far and away the most common place to have a contravariant type parameter).

Still, I think I favor inference. It's inline with what we do for regions and I think it will basically match up with user expectations.

@nikomatsakis
Copy link
Contributor Author

Do not forget: current region parameterization inference codes does not consider supertrait inheritance.

@graydon
Copy link
Contributor

graydon commented Mar 12, 2013

Can you be a little more concrete about use cases and current failure modes? I'm having a hard time picturing the implications of this.

@brson
Copy link
Contributor

brson commented Mar 31, 2013

Doesn't look like 0.6

@catamorphism
Copy link
Contributor

Nominating for milestone 3, feature-complete

@bblum
Copy link
Contributor

bblum commented Jun 13, 2013

@graydon, I ran into this today. I had something like:

struct Foo<'self> {
    field1: &'self Bar
    field2: Option<&'self Baz>,
}

...and I needed to update such a struct that I already had with a new value for field2, and it wouldn't let me intersect the lifetimes (even though the subtyping was safe, it needed them to be equal due to invariance). I have a FIXME in a local branch which should show up here when I push it.

As a workaround, I wrote a hand-monomorphized enum that's the same as option but didn't have a type parameter. (I can imagine that would be impossible for someone implementing a generic data structure, though.)

@pnkfelix
Copy link
Member

punting to later triage mtg with niko present.

@nikomatsakis
Copy link
Contributor Author

Related to #5781

@graydon
Copy link
Contributor

graydon commented Jul 11, 2013

accepted for feature-complete milestone

@nikomatsakis
Copy link
Contributor Author

@pcwalton thinks this isn't necessary for 1.0. I'm not sure I agree,
particularly because I encountered a lot of compilation errors trying
to fix the whole with the Self type parameter (referenced in my earlier comment).

@nikomatsakis
Copy link
Contributor Author

As part of the code for #4846 I implemented a general variance inference algorithm, but I do not yet apply it to type parameters.

nikomatsakis added a commit to nikomatsakis/rust that referenced this issue Nov 9, 2013
… old

region-parameterization/variance inference. We now compute variance for
type parameters but do not make use of it (most of the way towards rust-lang#3598).
@pnkfelix
Copy link
Member

nominating for discussion during triage when @nikomatsakis is present

@nikomatsakis
Copy link
Contributor Author

Marking as E-mentor -- this is almost entirely done. The variance inference is written. The declarations are written (in the form of marker types). The only thing missing is to update the type inferencer to consider the results of variance inference rather than treating all type parameters as invariant, and to update the vtable matching code in a similar way. Not exactly easy changes but not terribly hard with some pointers.

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

Assigning P-backcompat-lang, 1.0 blocker. (The current default of invariant is backwards compatible, but niko says there's some other interaction with trait-matching that niko says makes this more important to the point of being P-backcompat-lang.)

@pnkfelix pnkfelix added this to the 1.0 milestone Feb 6, 2014
@nrc
Copy link
Member

nrc commented Feb 6, 2014

cc me

@nikomatsakis
Copy link
Contributor Author

@edwardw I guess I don't know what problem you're encountering precisely. I know you sent me some gists but I never head time to read into them. Perhaps you can repost them here. It's hard to say what the options are without knowing what the problem is, it seems to me it's likely just a bug somewhere.

@edwardw
Copy link
Contributor

edwardw commented Mar 13, 2014

@nikomatsakis, I'd like to draw your attention to this line of the PR: combine.rs. Without it, the patched rustc won't be able to compile kinds.rs. The specific error messages are:

#[lang="covariant_type"]
#[deriving(Eq,Clone)]  // error: cannot determine a type for this expression: unconstrained type
pub struct CovariantType<T>;

Same error if manually implementing the Eq and Clone traits.

@edwardw
Copy link
Contributor

edwardw commented Mar 15, 2014

@nikomatsakis @nick29581 I think I figured out what are those mysterious bivariant. They come from nullary generic type. With that handled we are one step closer to fix this bug.

Since this is going to be a change with potentially big impact, do you have some specific tests in mind for it? Or some formerly ignored tests should be turned on as part of the fix?

@edwardw
Copy link
Contributor

edwardw commented Mar 15, 2014

All is worked out. #12828 has all the details and I think it fixes both this bug and #5781 :) Please review.

@pcwalton
Copy link
Contributor

pcwalton commented May 7, 2014

Nominating for removal from 1.0 milestone because the remaining soundness issue is covered by #5781 which is already milestoned.

@brson brson added P-high and removed I-nominated labels May 8, 2014
@brson brson modified the milestone: 1.0 May 8, 2014
@brson
Copy link
Contributor

brson commented May 8, 2014

P-high, not 1.0.

@rust-highfive
Copy link
Collaborator

This issue has been moved to the RFCs repo: rust-lang/rfcs#281

bors pushed a commit to rust-lang-ci/rust that referenced this issue May 15, 2021
RalfJung pushed a commit to RalfJung/rust that referenced this issue May 11, 2024
alloc: update comments around malloc() alignment

Also separate the C heap shims form the Windows heap shims; their guarantees aren't quite the same.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-type-system Area: Type system E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants