Skip to content

Do not ICE when interpreting a cast between non-monomorphic types #108790

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 7, 2023
Merged

Do not ICE when interpreting a cast between non-monomorphic types #108790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d7234a
Add test.
cjgillot Mar 5, 2023
858eab6
Do not ICE when casting polymorphic values.
cjgillot Feb 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 22 additions & 13 deletions compiler/rustc_const_eval/src/interpret/cast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

Pointer(PointerCast::ReifyFnPointer) => {
// All reifications must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just have a single one of these at the top of fn cast?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessarily wrong to cast partially polymorphic values, as long as the type parameters don't change the behaviour (layout / choice of unsize branch...). The checks up to this point are enough to make sure of this, without requiring monomorphic types.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the match on src.layout.ty.kind will ignore such leeway, so requires the type to be "monomorphic enough".

Copy link
Member

@RalfJung RalfJung Mar 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interpreter has no interest in supporting generic code. So IMO it'd make sense, and significantly simplify things, if we could do the check once at the top and then not think about the generic case any more -- rather than having to chase ICEs each time this code gets refactored.


// The src operand does not matter, just its type
match *src.layout.ty.kind() {
ty::FnDef(def_id, substs) => {
// All reifications must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;

let instance = ty::Instance::resolve_for_fn_ptr(
*self.tcx,
self.param_env,
Expand Down Expand Up @@ -100,12 +100,12 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

Pointer(PointerCast::ClosureFnPointer(_)) => {
// All reifications must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;

// The src operand does not matter, just its type
match *src.layout.ty.kind() {
ty::Closure(def_id, substs) => {
// All reifications must be monomorphic, bail out otherwise.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;

let instance = ty::Instance::resolve_closure(
*self.tcx,
def_id,
Expand Down Expand Up @@ -359,8 +359,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
let val = Immediate::new_dyn_trait(ptr, vtable, &*self.tcx);
self.write_immediate(val, dest)
}

_ => {
// Do not ICE if we are not monomorphic enough.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;
ensure_monomorphic_enough(*self.tcx, cast_ty)?;

span_bug!(
self.cur_span(),
"invalid pointer unsizing {:?} -> {:?}",
Expand Down Expand Up @@ -404,12 +407,18 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}
Ok(())
}
_ => span_bug!(
self.cur_span(),
"unsize_into: invalid conversion: {:?} -> {:?}",
src.layout,
dest.layout
),
_ => {
// Do not ICE if we are not monomorphic enough.
ensure_monomorphic_enough(*self.tcx, src.layout.ty)?;
ensure_monomorphic_enough(*self.tcx, cast_ty.ty)?;

span_bug!(
self.cur_span(),
"unsize_into: invalid conversion: {:?} -> {:?}",
src.layout,
dest.layout
)
}
}
}
}
15 changes: 15 additions & 0 deletions tests/ui/mir/unsize-trait.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Check that the interpreter does not ICE when trying to unsize `B` to `[u8]`.
// This is a `build` test to ensure that const-prop-lint runs.
// build-pass

#![feature(unsize)]

fn foo<B>(buffer: &mut [B; 2])
where B: std::marker::Unsize<[u8]>,
{
let buffer: &[u8] = &buffer[0];
}

fn main() {
foo(&mut [[0], [5]]);
}