Conversation
bbfea47 to
d8966af
Compare
This comment has been minimized.
This comment has been minimized.
src/test/mir-opt/boxy_thing/otherwise_refinements.foo.BoxyThing.diff
Outdated
Show resolved
Hide resolved
a53e13e to
9f4beb3
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
03b6261 to
a5bcda9
Compare
This comment has been minimized.
This comment has been minimized.
oli-obk
left a comment
There was a problem hiding this comment.
Probably a good idea to squash some (most?) commits
There was a problem hiding this comment.
Probably need to do the same for AddressOf. Also immut borrows can be a problem with interior mut
There was a problem hiding this comment.
I don't think interior mut is problematic though im having trouble explaining why I think that 🤔
There was a problem hiding this comment.
I had the same reaction as you, but couldn't explain it. Some thinking lead me to believe the reason is that interior mutability means you can only access the data via methods, so you lose all refinements anyway.
There was a problem hiding this comment.
I am going to just clear refinements on AddressOf even when its not mutable to be on the safe side. I think that even though it seems unproblematic i don't trust it 😅
There was a problem hiding this comment.
Looking at how UnsafeCell is implemented in core I think that its probably fine right now given the combination of no refinements on projections and only being able to get at the inner value through methods. This all feels rather sketchy though...
There was a problem hiding this comment.
The current implementation also makes implicit assumption that an assignment invalidates all existing borrows.
There was a problem hiding this comment.
is this assumption not correct
6364ac8 to
19898d6
Compare
| for refine in self.0.iter_mut() { | ||
| *refine = Some(RefineRange::from(0..=(u128::MAX))); | ||
| } |
There was a problem hiding this comment.
| for refine in self.0.iter_mut() { | |
| *refine = Some(RefineRange::from(0..=(u128::MAX))); | |
| } | |
| self.0.fill(Some(RefineRange::from(0..=u128::MAX))); |
|
my understanding is that this is now: @rustbot label -S-waiting-on-review +S-waiting-on-author (btw I miss the meaning implied by the title of this PR) |
|
☔ The latest upstream changes (presumably #91469) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Ping from triage: |
|
just gonna close this because im tired and dont want to think about this PR lol |
r? @oli-obk