phobos
phobos copied to clipboard
std.math: Add RealRep union for accessing floating point bits via a union.
As I'd like to see this move forwards, but am in the middle of finishing up work on new ports (making sure that 128-bit is working properly for both IEEE and IBM reals) so can't really commit at the moment.
This is a slightly more conservative version of #4336. As far as I can see, GDC at least produces identical assembly in the conversion from old to new.
Should allow the bigger refactorings in the other PR to be added gracefully. @tsbockman - would you agree?
Thanks for your pull request, @ibuclaw!
Bugzilla references
Your PR doesn't reference any Bugzilla issue.
If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.
Testing this PR locally
If you don't have a local development environment setup, you can use Digger to test this PR:
dub fetch digger
dub run digger -- build "master + phobos#5825"
Unless someone has added floating-point union
support to DMD since I last checked, this PR has the same problem which stalled mine: it breaks some parts of std.math
which currently work in CTFE.
It could be re-written to emulate a union
with a struct
and properties encapsulating some pointer code, but given that the response to my efforts at extending the CTFE pointer repainting capabilities to support all basic operations was fairly hostile, it didn't seem worth my time.
I think the compiler team needs to pick a long-term solution to the floating-point CTFE issue before it's worth refactoring std.math
.
it breaks some parts of std.math which currently work in CTFE.
I don't particularly buy that, as many functions that do pointer casting are not CTFE-able anyway.
Typical getter/setters look like this.
ushort *vu = cast(ushort *)&f;
vu[EXPPOS_SHORT] = 0;
The value of vu[0]
might be inferable but any other index certainly isn't.
Specifically, this will break isInfinity
and isNaN
at CTFE for double
and float
:
import std.math, std.stdio;
void main() {
enum float x = float.nan;
enum a = isInfinity(x);
enum b = isNaN(x);
writeln(a, ", ", b);
enum double y = double.infinity;
enum c = isInfinity(y);
enum d = isNaN(y);
writeln(c, ", ", d);
}
Admittedly, these can both be reimplemented in a CTFE-compatible way by the user easily enough:
bool isInfinity(X)(X x) { return x > X.max || x < -X.max; }
bool isNaN(X)(X x) { return x != x; }
But, it is still a breaking change. If that's considered acceptable, then why not just go with my original pull request?
I'm in the middle of testing ieeeQuadruple and ibmExtended, and finding bugs in both. I'd want to make sure that is complete before introducing yours. To make sure we start from a working implementation before the redesign.
That makes sense.
ping @ibuclaw
Please rebase?
Rebased.
@ibuclaw maybe it's better if you rebase yourself this time?
Rebased, but I'll have to double-check that ibmExtended is still fine (did some educated guesswork on reimplementing the floorImpl I added in the new format).
@ibuclaw would need another rebase.