-
-
Notifications
You must be signed in to change notification settings - Fork 411
Conversation
|
Thanks for your pull request and interest in making D better, @thewilsonator! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour 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 locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + druntime#2135" |
|
LGTM. |
|
@thewilsonator Are you still interested in pursuing this? It appears there are |
|
Yes, what mismatches are there (I've just got back from Europe so I'm not thinking properly yet)? |
See the output of the test suite. |
|
What's the status of this PR? The dmd PR has been merged already... |
|
Just rebased, lets see what happens. |
| float f = 1.1f; | ||
| double d = 1.1; | ||
| real r = 1.1L; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ibuclaw was there any reason these need to be static? This makes the unittest impure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To force the code generator (not CTFE) to do the work. Practically the same is done in the test suite, so could remove if you really must.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll remove it as part of #2871
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That PR seems to have offended the gods of continuous integration, so I'd rather that be done here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added approxEqual, so the Win32 targets should now be happy with their inaccuracies.
|
Needs dlang/phobos#7461 |
Is now merged, this should be good to go. |
druntime part dlang/dmd#7995