Skip to content

handle unsized by-ref values#1644

Open
0xmuon wants to merge 2 commits intorust-lang:mainfrom
0xmuon:fix3
Open

handle unsized by-ref values#1644
0xmuon wants to merge 2 commits intorust-lang:mainfrom
0xmuon:fix3

Conversation

@0xmuon
Copy link
Copy Markdown

@0xmuon 0xmuon commented Apr 14, 2026

added handling for unsized by-ref values (CValueInner::ByRef(_, Some(meta))) in CValue::value_field and removes the remaining todo!() crash paths in src/value_and_place.rs. For write_cvalue from an unsized by-ref into a sized destination, it falls back to copying the sized prefix (destination’s fixed size) to avoid hard failures.

CValueInner::ByRef(_, Some(_)) => todo!(),
CValueInner::ByRef(from_ptr, Some(_extra)) => {
// Unsized values shouldn't normally be written into sized places. However,
// if this happens, we can still copy the sized prefix using the destination layout's fixed size.
Copy link
Copy Markdown
Member

@bjorn3 bjorn3 Apr 14, 2026

Choose a reason for hiding this comment

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

View changes since the review

If an IR invariant is violated, we should fail loudly, not silently do something that seems roughly correct, but almost certainly causes hard to debug corruption.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yup agreed! will correct

@0xmuon
Copy link
Copy Markdown
Author

0xmuon commented Apr 14, 2026

Is “copy sized prefix when writing from ByRef(_, Some(meta)) into a sized place” fallback acceptable or would you prefer we span_bug! there instead to enforce that such writes never happen?

} else {
CValue::by_ref(field_ptr, field_layout)
}
}
Copy link
Copy Markdown
Member

@bjorn3 bjorn3 Apr 14, 2026

Choose a reason for hiding this comment

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

View changes since the review

The only places where value_field is used don't use unsized values afaik. It can always be implemented later if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants