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

Implement type ascription. #23773

Closed
wants to merge 1 commit into from
Closed

Implement type ascription. #23773

wants to merge 1 commit into from

Conversation

Ryman
Copy link
Contributor

@Ryman Ryman commented Mar 27, 2015

Rebased #21836

cc @eddyb (didn't see any reply on irc), @nrc

@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@nrc
Copy link
Member

nrc commented Mar 27, 2015

@Ryman I'm not sure if you have already, but I think we need some changes so that we do not attempt to coerce in a type ascription expression when the expression is in reference (lvalue) context. This is required for soundness. We also need tests for that. (See RFC 987 for more details).

@nrc
Copy link
Member

nrc commented Mar 27, 2015

And thanks for doing the rebase!

@Ryman
Copy link
Contributor Author

Ryman commented Mar 27, 2015

@nrc Only a quick rebase and cfail/rpass run, hadn't noticed the requirements change other than the loss of 'ascriptions in patterns'. Hope it's still useful for @eddyb!

@eddyb
Copy link
Member

eddyb commented Mar 27, 2015

@Ryman thanks!
@nrc I don't even know how to tell whether something is an lvalue or not, this early during typeck.
There is a function in middle::ty which can return a few kinds of lvalue/rvalue kinds, used from trans, not sure if it's applicable here.

@nrc
Copy link
Member

nrc commented Mar 30, 2015

@eddyb: Using those fns in middle::ty would be my first bet. Otherwise, it is a strictly syntactic property - you just need to know the context of the expression. If none of that works out, then you could check later in type checking (mem categorization, I guess) that if you find an ascription expression, and it's an lvalue, then check to see if there is an adjustment on that expression, if there is, error out at that stage.

@eddyb
Copy link
Member

eddyb commented Mar 30, 2015

The issue was related to lifetime invariance in mutable lvalues, wasn't it?

I do wonder what we do right now for (&mut &mut lvalue): T and (&mut *&mut &mut lvalue): T.
I expect that we either propagate the expected type and coerce the inner &mut in both case, or we don't in either.
If we do, then we either have a pre-existing soundness hole to fix or it works due to regionck and borrowck.
If we don't, then we can reuse the same decision for type ascription.

I haven't had the chance to experiment with this fully.

Oh and I didn't think of unsizing coercions on a pointer-type lvalue - but that would consume the lvalue, so it shouldn't have bad interactions with ascription.

@eddyb
Copy link
Member

eddyb commented Mar 30, 2015

Some quick playpen experimentation suggests we do propagate expected types down, but we don't attempt coercions on lvalues:
http://is.gd/Hsa3ow
http://is.gd/Gu75Ca
http://is.gd/9fXuIt

@nikomatsakis
Copy link
Contributor

@eddyb that sounds right. In particular, the & and &mut operators don't do coercions.

@bors
Copy link
Contributor

bors commented Apr 7, 2015

☔ The latest upstream changes (presumably #23857) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

After some discussion with @nrc conclusion was that I should take over this PR too.

@alexcrichton
Copy link
Member

Closing in favor of an upcoming rebase by @nikomatsakis!

@cristicbz
Copy link
Contributor

I recently remembered that type ascription was supposed to land "any minute now" some time ago, is this still going to happen?

PS: Also may I suggest not closing issues in favour of upcoming rebases until the latter are actually opened (just so as not to have things fall through cracks). Not trying to be passive aggresive, just a genuine suggestion!

@nrc
Copy link
Member

nrc commented Jun 29, 2015

ping @nikomatsakis who was going to do the rebase

@pnkfelix
Copy link
Member

@alexcrichton @cristicbz

I can see why we would close a PR if e.g. its functionality is going to be provided in a different manner (so that people do not waste their time reviewing code that isn't even close to what is planned to be added).

But closing a PR based solely on an anticipated future rebase that has not been opened does seem misguided. After all, what if someone else were willing to do the rebase in the meantime?

(In other words, I think that closing this PR may have been an error.)

@alexcrichton
Copy link
Member

I'm fine reopening, I'm just trying to keep the queue clear!

@apasel422
Copy link
Contributor

Is someone working on the rebase of this?

@nikomatsakis
Copy link
Contributor

I was supposed to, but I haven't had the time. I'd be happy to mentor an
attempted rebase, though.

On Wed, Oct 7, 2015 at 3:42 PM, Andrew Paseltiner notifications@github.com
wrote:

Is someone working on the rebase of this?


Reply to this email directly or view it on GitHub
#23773 (comment).

@strega-nil
Copy link
Contributor

@nikomatsakis I'd like to attempt a rebase. Would you like to mentor
me? :P

@nikomatsakis
Copy link
Contributor

I would be happy to, but there has been someone doing the rebase that I've
been talking to on IRC. I haven't heard from them lately though, I should
check in.

On Sun, Oct 25, 2015 at 7:44 AM, Nicholas notifications@github.com wrote:

@nikomatsakis https://github.com/nikomatsakis I'd like to attempt a
rebase. Would you like to mentor
me? :P


Reply to this email directly or view it on GitHub
#23773 (comment).

@apasel422
Copy link
Contributor

I had been working on the rebase, but haven't been able to work on it recently, so @GBGamer is welcome to take it over.

@nikomatsakis
Copy link
Contributor

Ah, OK. Did you have an intermediate branch that might be worth looing at?
@GBGamer, feel free to ping me on IRC to discuss.

On Mon, Oct 26, 2015 at 2:11 PM, Andrew Paseltiner <notifications@github.com

wrote:

I had been working on the rebase, but haven't been able to work on it
recently, so @GBGamer https://github.com/GBGamer is welcome to take it
over.


Reply to this email directly or view it on GitHub
#23773 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.