-
Notifications
You must be signed in to change notification settings - Fork 39
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.
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
mir: lower cast
with MIR pass
#1411
base: devel
Are you sure you want to change the base?
Conversation
Summary ======= When compiling for the C backend, use a MIR pass to lower `cast`s between non-integer/non-pointer types into `nimCopyMem`. Details ======= * the `lowerCast` does the lowering, with the decision whether to use `nimCopyMem` based on the lowered MIR type * handling of `float` and aggregate type casts is removed from `cgen` * `cgen` used type-punning through a union for such casts, which is possible, but not as easy to do with a MIR pass, so `nimCopyMem` (i.e., `memcpy`) is used instead The copy always copies size-of-destination number of bytes.
There seems to be some |
That doesn't sound good to me, truncation is a perfectly valid use case for different size casts, especially when integer narrowing conversions are range checked and we wanted to bypass that. EDIT: It does make some sense to ban such conversions between objects, though. But then we do acknowledge that |
You're right, my statement was too broad. What happens in the truncation case is easy to explain, understand, implement, and it's useful, and therefore I think it should generally stay allowed. I do think it's better to disallow "expanding" casts, and I'm also starting to think that it'd be better to not make an exception for numeric types. Ultimately, I think bit-casting from smaller to larger types is less safe (i.e., the excess bytes/bits need to take on some defined or undefined state) than the other two cases (truncating and casting between equal-sized types) and also usually not what one wants when casting, which is why I think it makes sense to disallow it. Making what happens more explicit (either via a It's not a very strong opinion however, and as long as such casts are lowered into memory copies early enough, I don't have too much of a problem with them. |
I do agree that small -> large is very unsafe. They don't make that much sense for pointers (except for inheritance) and surely don't make much sense for a bit cast.
There's one edge case that is a bit lengthier to do if this is not allowed: conversion between One of these days we might have to reformulate how integer conversion is done to make them easier to use when implementing algorithms, but that's a conversation for later. |
i disagree reduce degrees of freedom to something that is unsafe at every way. I think to reduce unsafety on my problem with |
Summary
When compiling for the C backend, use a MIR pass to lower
cast
sbetween non-integer/non-pointer types into
nimCopyMem
.Details
lowerCast
does the lowering, with the decision whether to usenimCopyMem
based on the lowered MIR typefloat
and aggregate type casts is removed fromcgen
cgen
used type-punning through a union for such casts, which ispossible, but not as easy to do with a MIR pass, so
nimCopyMem
(i.e.,
memcpy
) is used insteadThe copy always copies size-of-destination number of bytes.
Notes for Reviewers
MemCopy
MIR operator at one pointcast
ing between different-sized types should become an error