Inexact and/or Wrong `{From,To}Primitive`

[lifthrasiir]

Motivating examples:

(100_0000_0000_0000_2345u64).to_f64(); // => Some(1000000000000002244f64)
(42.5f64).to_u8(); // => Some(42u8)
(42.5e80f64).to_u8(); // => Some(0u8)
(42.5e80f64).to_i32(); // => Some(-2147483648i32)

Note that the last two results can change depending on your optimization level. (Is it another issue?) It seems that it's due to the constant propagation across inlined function calls.

Currently we check for such cases in int to int and float to float conversion, but we don't check (and solely rely on casting) in int to float and float to int conversion. This can cause the inexact computation in two ways: unwanted truncation, and failed bound check. Four possible decisions have their pros and cons respectively, but should be decided and documented.

[huonw]

(100_0000_0000_0000_2345u64).to_f64(); // => Some(1000000000000002244f64)

This appears to just be floating point precision, although (python3):

>>> int(float(1000000000000002345))
1000000000000002304

so we're not doing the best job possible.

(Personally, I'd think that returning None as soon as any precision is lost for int -> float would make these traits much less useful... it's just a hazard of working with floating point.)

The others are concerning though.

[huonw]

Also,

Note that the last two results can change depending on your optimization level. (Is it another issue?) It seems that it's due to the constant propagation across inlined function calls.

Is likely because the result is undefined if the value doesn't fit in result type (and clearly 42.5e80 doesn't fit in a u8 or i32).

The ‘fptoui‘ instruction converts its floating point operand into the nearest (rounding towards zero) unsigned integer value. If the value cannot fit in ty2, the results are undefined.

docs: float → unsigned, float → signed

[lifthrasiir]

@huonw I agree that the strict exactness would make the methods less useful, but this fact itself should be documented somehow. (Or, if we are extra careful about this issue, we can add to_i32_exact() etc. Aren't we already have traits for saturating arithmetics?) For the undefined behavior for fptoui, it looks like a copy of C/C++'s semantics (naturally); in Rust we already have bound-checking conversion methods (e.g. 65536u.to_i16() == None) so it is more important to be consistent.

[huonw]

in Rust we already have bound-checking conversion methods (e.g. 65536u.to_i16() == None) so it is more important to be consistent.

Yep, I definitely agree that they should be bounds-checked, I'd guess that it was just @erickt overlooking the float conversions.

[nikomatsakis]

As @thestinger pointed out, undefined results -- particularly of
integral type -- are much worse than previously believed and can lead
to bounds check violations and so forth, so I guess we do need to fix
this.

[nikomatsakis]

Nominating.

[pcwalton]

We need -fwrapv.

[pnkfelix]

pcwalton recommends we adopt an approach of implementing the equivalent of:

-fwrapv
    This option instructs the compiler to assume that signed arithmetic overflow of addition, subtraction and 
multiplication wraps around using twos-complement representation. This flag enables some optimizations and 
disables others. This option is enabled by default for the Java front end, as required by the Java language 
specification.

[pnkfelix]

Accepted for P-high; it is a potential (though at least known) soundness hole, and thus a security bug. But we are permitted to still leave such things for post 1.0 (they are just still high priority in that context).

[alexcrichton]

It appears that to support -fwrapv we would need to simply build arithmetic instructions differently. The clang implementation simply calls the corresponding LLVM builder function, and I imagine that we would do something similar.

I'm unsure of whether we'd want to do this in all arithmetic, though, or only in some cases.

[treeman]

Visiting for triage, still relevant.

[bombless]

Luckily ToPrimitive was gone so we don't have this problem in std any more.

[arielb1]

The last of these is just a duplicate of #10184, the others are expected behaviour.