fix(interp): Treat UnsafeBinder as Compound Type in try_visit_primitive#153458
fix(interp): Treat UnsafeBinder as Compound Type in try_visit_primitive#153458TKanX wants to merge 1 commit intorust-lang:mainfrom
UnsafeBinder as Compound Type in try_visit_primitive#153458Conversation
rustbot
added
S-waiting-on-author
rustbot
added
S-waiting-on-review
|
can you explain a bit your choice of this match arm? |
Since layout.rs erases bound regions and delegates entirely to the inner type, I think returning Happy to be corrected if there's something specific. |
Enselic
added
the
A-const-eval
|
r? rust-lang/compiler |
| | ty::Dynamic(..) | ||
| | ty::Closure(..) | ||
| | ty::Pat(..) | ||
| | ty::UnsafeBinder(..) |
There was a problem hiding this comment.
Sorry, I am not super sure about the decision here. However, I would like to leave a long overdue opinion and I would like to take this opportunity to learn more about the UnsafeBinder semantics here as well.
In my very rudimentary understanding of an unsafe<..> type and the primitivity property in CTFE is so that it transcends through the binder. For instance the primitivity of unsafe<'a> &'a T is delegated to &'a T, which depends on how well-behaving this underlying reference is. So does this make sense?
| #![allow(incomplete_features)] | ||
|
|
||
| struct ThinDst { | ||
| b: unsafe<> (), |
There was a problem hiding this comment.
... so if a raw pointer, instead of a unit, is behind the binder, we probably should also check the reference here, or?
There was a problem hiding this comment.
While this was the minimal repro for the ICE, fixing the ICE needs to come with some extensive testing of the feature within const validation.
So having tests where a reference behind an unsafe binder points to the wrong memory (e.g. a fn) or points to uninit memory even tho the value can't be uninit if it were a normal reference. There's probably more fun edge cases to handle here
8 participants
Summary:
Fixes ICE in const eval validity checking when a value contains an
UnsafeBinder-typed field.try_visit_primitiveinvalidity.rshadty::UnsafeBinder(_) => todo!("FIXME(unsafe_binder)"): a placeholder left by the initial unsafe binders type system implementation (9a1c5eb).The fix is to remove the
todo!()and moveUnsafeBinderinto the compound types arm.Closes #153362
r? @dingxiangfei2009
cc @matthiaskrgr