Uh oh!
There was an error while loading. Please reload this page.
Refactor Adjust and CastKind - #59987
Conversation
rust-highfive
commented
Apr 15, 2019
(rust_highfive has picked a reviewer for you, use r? to override) |
saleemjaffer
commented
Apr 15, 2019
r? @oli-obk |
rust-highfive
commented
Apr 15, 2019
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
saleemjaffer
commented
Apr 15, 2019
@oli-obk This is not yet ready. I have just made the changes to |
94d02eb to
5be6b0bCompareUh oh!
There was an error while loading. Please reload this page.
rust-highfive
commented
Apr 15, 2019
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
saleemjaffer
commented
Apr 16, 2019
@oli-obk I have done a basic refactor of |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
rust-highfive
commented
Apr 16, 2019
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
rust-highfive
commented
Apr 16, 2019
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
oli-obk
left a comment
There was a problem hiding this comment.
Just a nit, then this is ready to go!
| Adjust::Pointer(PointerCast::UnsafeFnPointer) => { | ||
| ExprKind::UnsafeFnPointer { source: expr.to_ref() } | ||
| Adjust::Pointer(PointerCast::Unsize) => { | ||
| if let ExprKind::Block { body } = expr.kind { |
There was a problem hiding this comment.
you lost the comment about looking at the comment on Adjust::Deref
There was a problem hiding this comment.
// See the above comment for Adjust::Deref
I thought the comment was misplaced. Why do we mention about Adjust::Deref in Adjust::Unsize?
Also, it does not seem like the existing "above comment" spoke about Adjust::Deref.
There was a problem hiding this comment.
It's about the comment in line 94. This code exists solely to improve diagnostics. Maybe pull it out into a function instead of duplicating the logic?
There was a problem hiding this comment.
Added a new function adjust_span for this.
oli-obk
commented
Apr 16, 2019
You also need to update |
Uh oh!
There was an error while loading. Please reload this page.
saleemjaffer
commented
Apr 18, 2019
@oli-obk Made the fixes. |
oli-obk
commented
Apr 20, 2019
@bors r+ Thanks! |
bors
commented
Apr 20, 2019
📌 Commit a2f8269 has been approved by |
bors
commented
Apr 20, 2019
Refactor Adjust and CastKind fixes#59588
bors
commented
Apr 20, 2019
☀️ Test successful - checks-travis, status-appveyor |
…casts-because-that-sounds-way-to-general-aaaa, r=oli-obk Rename `adjustment::PointerCast` and variants using it to `PointerCoercion` It makes it sounds like the `ExprKind` and `Rvalue` are supposed to represent all pointer related casts, when in reality their just used to share a little enum variants. Make it clear there these are only coercions and that people who see this and think "why are so many pointer related casts not in these variants" aren't insane. This enum was added in rust-lang#59987. I'm not sure whether the variant sharing is actually worth it, but this at least makes it less confusing. r? oli-obk
…casts-because-that-sounds-way-to-general-aaaa, r=oli-obk Rename `adjustment::PointerCast` and variants using it to `PointerCoercion` It makes it sounds like the `ExprKind` and `Rvalue` are supposed to represent all pointer related casts, when in reality their just used to share a little enum variants. Make it clear there these are only coercions and that people who see this and think "why are so many pointer related casts not in these variants" aren't insane. This enum was added in rust-lang#59987. I'm not sure whether the variant sharing is actually worth it, but this at least makes it less confusing. r? oli-obk
fixes#59588