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

Refactor coercions/adjustments #24261

Merged
merged 4 commits into from
Apr 14, 2015
Merged

Refactor coercions/adjustments #24261

merged 4 commits into from
Apr 14, 2015

Conversation

nrc
Copy link
Member

@nrc nrc commented Apr 10, 2015

@eddyb's preparatory work for DST coercions

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@nrc so I reviewed the code. I think it's good, but I have a couple of nits. For one, I think there is a simpler explanation for how AutoderefRef works. This seems to both be logical and fit what trans does, and also makes Box not really a special case. Anyway, I wrote this up here:

nikomatsakis@3f310ae

that patch also includes a simplification to the ExprUseVisitor based on this idea that makes it less hacky. Thoughts?

r+ modulo these nits

@nrc
Copy link
Member Author

nrc commented Apr 10, 2015

Comments inline on your patch. I'm not sure I agree with you explanation 100%, but maybe I misunderstood something last night. It might be that there are just two views on this - the type system's and trans's and they're fundamentally a bit different.

The ExprUseVisitor change looks nice, I can't comment on whether I think it will work :-p

@bors
Copy link
Contributor

bors commented Apr 12, 2015

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

@nikomatsakis
Copy link
Contributor

@nrc I responded inline. Seems like we see the code a bit differently...but I still stand by my interpretation ;) maybe we can discuss it online (either IRC or vidyo), or maybe @eddyb can weight in as to how he sees it.

@nrc
Copy link
Member Author

nrc commented Apr 13, 2015

@bors r=nikomatsakis 231b1d0

@nrc
Copy link
Member Author

nrc commented Apr 13, 2015

Rebased and added Niko's changes (with a little reformatting).

@bors
Copy link
Contributor

bors commented Apr 14, 2015

⌛ Testing commit 231b1d0 with merge a3f80bb...

@bors
Copy link
Contributor

bors commented Apr 14, 2015

💔 Test failed - auto-win-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Mon, Apr 13, 2015 at 5:04 PM, bors [email protected] wrote:

[image: 💔] Test failed - auto-win-32-opt
http://buildbot.rust-lang.org/builders/auto-win-32-opt/builds/4146


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

@bors
Copy link
Contributor

bors commented Apr 14, 2015

⌛ Testing commit 231b1d0 with merge 958a90c...

@bors
Copy link
Contributor

bors commented Apr 14, 2015

💔 Test failed - auto-linux-64-nopt-t

@nrc
Copy link
Member Author

nrc commented Apr 14, 2015

@bors r=nikomatsakis b35a587

@bors
Copy link
Contributor

bors commented Apr 14, 2015

⌛ Testing commit b35a587 with merge 47551b5...

bors added a commit that referenced this pull request Apr 14, 2015
@bors bors merged commit b35a587 into rust-lang:master Apr 14, 2015
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.

5 participants