fix ICE in CMSE type validation#130064
Conversation
rustbot
added
S-waiting-on-review
|
HIR ty lowering was modified cc @fmease |
| fn is_valid_cmse_inputs<'tcx>( | ||
| tcx: TyCtxt<'tcx>, | ||
| fn_sig: ty::PolyFnSig<'tcx>, | ||
| fn_sig: ty::FnSig<'tcx>, |
There was a problem hiding this comment.
One tweak -- can you change this back to PolyFnSig and do the binder instantiate call inside this fn?
| for (index, arg_def) in fn_sig.inputs().iter().enumerate() { | ||
| let layout = tcx.layout_of(ParamEnv::reveal_all().and(*arg_def.skip_binder()))?; | ||
| for (index, ty) in fn_sig.inputs().iter().enumerate() { | ||
| let layout = tcx.layout_of(ParamEnv::reveal_all().and(*ty))?; |
There was a problem hiding this comment.
Side-note: this param-env is not right, so it'll probably ICE in other cases...
There was a problem hiding this comment.
yeah, #130104 was just reported and is not fixed by this branch (currently). How do we get a correct ParamEnv here?
There was a problem hiding this comment.
actually that second ICE is due to encountering Infer being considered a bug in layout computation
rust/compiler/rustc_ty_utils/src/layout.rs
Lines 677 to 683 in 263a3ae
Moving ty::Infer(_) to the ty::Placeholder(..) branch fixes that problem, but might hide bugs in other cases?
There was a problem hiding this comment.
yeah, that is 100% not the right fix. the problem here is that we need to delay the calculation of these layouts until after type-checking is done for types that show up.
There was a problem hiding this comment.
here #127814 (comment) is where some discussion around the current design happened. Is there an existing way to perform that traversal after type checking to perform this layout check?
|
pls in the future squash changes like this into one commit, a one-liner is not really worth the additional commit lol |
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
folkertdev
added
the
F-abi_cmse_nonsecure_call
4 participants
fixes #129983
tracking issue: #81391
r? @compiler-errors