Skip to content

Conversation

@JackStouffer
Copy link
Contributor

This function

  1. Results in link errors if you're not using DM's C runtime
  2. Is non-standard
  3. Is covered by round and friends

Side note: all of these platform specific math functions which share the C names feel a lot like PHP. This is not how these would be designed if std.math was to be written today, so any instance of these that we can remove/redesign, the better.

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @JackStouffer!

Bugzilla references

Auto-close Bugzilla Severity Description
18693 normal std.math.rndtonl and core.math.rndtonl result in link errors

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#6429"

@JackStouffer
Copy link
Contributor Author

Looks like ddox is mis-parsing the unittest block and including the next function signature

@wilzbach
Copy link
Contributor

wilzbach commented Apr 9, 2018

Both at the time of deprecation and removal, a changelog entry must be made. This
changelog entry should have a short motivation for the deprecation (or removal)
and should describe which steps can be taken by the user to upgrade their codebase.

(from dlang/DIPs#108)

@wilzbach wilzbach added the Review:Needs Changelog A changelog entry needs to be added to /changelog label Apr 9, 2018
Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Side note: all of these platform specific math functions which share the C names feel a lot like PHP. This is not how these would be designed if std.math was to be written today, so any instance of these that we can remove/redesign, the better.

Fully agreed. std.math would look quite differently today.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 9, 2018

@andralex could we get your approval on this one? Thanks!

@JackStouffer JackStouffer removed the Review:Needs Changelog A changelog entry needs to be added to /changelog label Apr 9, 2018
@JackStouffer JackStouffer force-pushed the issue18693 branch 2 times, most recently from f32aefe to 0c2173b Compare April 10, 2018 13:58
Copy link
Member

@andralex andralex left a comment

Choose a reason for hiding this comment

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

If semantics are not identical, please specify differences in the deprecation message and changelog. Thanks!

@JackStouffer
Copy link
Contributor Author

assert(rndtonl(1.0) is -real.nan);

does this function even work at all

@JackStouffer
Copy link
Contributor Author

does this function even work at all

Nope.

@dlang-bot dlang-bot merged commit 813327d into dlang:master Apr 10, 2018
@JackStouffer JackStouffer deleted the issue18693 branch April 10, 2018 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants