-
Notifications
You must be signed in to change notification settings - Fork 390
rustup to e7c23ab933ebc1f205c3b59f4ebc85d40f67d404 #1716
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
Conversation
let place = this.deref_operand(place)?; | ||
|
||
// make sure it fits into a scalar; otherwise it cannot be atomic | ||
let val = this.read_scalar_atomic(place, atomic)?; | ||
let val = this.read_scalar_atomic(&place, atomic)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is it deref_operand(place)
a few lines above but read_scalar_atomic(&place
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first ref place
binding is being shadowed by let place = this.deref_operand(place)?;
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, good point, thanks.
let old = this.allow_data_races_mut(|this| this.read_scalar(place.into()))?; | ||
this.allow_data_races_mut(|this| this.write_scalar(new, place.into()))?; | ||
let old = this.allow_data_races_mut(|this| this.read_scalar(&place.into()))?; | ||
this.allow_data_races_mut(|this| this.write_scalar(new, &(*place).into()))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
&place
vs &(*place)
seems inconsistent here, they are doing the same thing, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First constructs OpTy
which has From<&MPlaceTy>
, the other construct PlaceTy
which has only From<MPlaceTy>
(it was unnecessary for rustc).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't realize you were adding new From
instances as well.
Thanks a lot, this must have been a lot of work. :) We can land this as-is, but I am confused by a few things -- see my questions above. |
@bors r+ |
📌 Commit 0eb3414 has been approved by |
☀️ Test successful - checks-actions |
No description provided.