Skip to content
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

Fix Issue 9298 - some std.math functions not implemented for Win64 #7360

Merged
merged 1 commit into from
May 24, 2020

Conversation

e10s
Copy link
Contributor

@e10s e10s commented Jan 23, 2020

To be precise, for -m32mscoff / -m64.

@e10s e10s requested a review from ibuclaw as a code owner January 23, 2020 09:17
@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 23, 2020

Thanks for your pull request and interest in making D better, @e10s! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

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 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 run digger -- build "master + phobos#7360"

@PetarKirov
Copy link
Member

CC @kinke @rainers

@kinke
Copy link
Contributor

kinke commented Jan 23, 2020

Please don't auto-merge - DMD still ships with VS 2010 import libs, which doesn't provide these functions [edit: IIRC; MSDN doesn't provide any infos for versions < 2015 anymore].

@e10s
Copy link
Contributor Author

e10s commented Jan 23, 2020

Hmm, I was under an illusion. I have to test with -mscrtlib=msvcr100 switch...

This depends on dlang/installer#377?

@kinke
Copy link
Contributor

kinke commented Jan 23, 2020

Heh yeah, great that you've found it yourself. :) - That, in combination with a switch to the VS 2015/UCRT libs, should work; but that requires testing and some further adaptations wrt. linker symbol aliases (which would already be in LDC's Phobos and just need to be moved here upstream).

Btw, LDC's libs are a bit newer than those in dlang/installer#377, see https://github.com/ldc-developers/mingw-w64-libs (where you can also download the generated import libs directly).

@thewilsonator
Copy link
Contributor

Is this good to go?

@dukc
Copy link
Contributor

dukc commented Apr 27, 2020

@kinke is this good to go?

@kinke
Copy link
Contributor

kinke commented Apr 27, 2020

AFAIK, DMD now defaults to the MinGW-based libs for the VS2013 runtime (which is already a big improvement over the previous VS2010 ones). So someone needs to check whether these functions are available in VS2013.

@thewilsonator
Copy link
Contributor

cc @rainers do you know?

@rainers
Copy link
Member

rainers commented May 21, 2020

Finally tested now: std.math unittests in this PR link and run fine with -mscrtlib=msvcrt120 (but not earlier), so should be good to go.

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.

7 participants