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

More header fixes #126

Merged
merged 3 commits into from
Mar 8, 2016
Merged

More header fixes #126

merged 3 commits into from
Mar 8, 2016

Conversation

nalimilan
Copy link
Contributor

The first commit fixes a warning on Fedora rawhide with GCC6. But we should check that this works on Windows.

The second commit is just a cleanup.

@nalimilan nalimilan mentioned this pull request Mar 3, 2016
@nalimilan
Copy link
Contributor Author

I've enabled OS X on Travis CI in a branch, and the build works (though tests fail): https://travis-ci.org/JuliaLang/openlibm/jobs/113449199

But there's still another warning about redefining __pure2, which is already defined by sys/cdefs.h. I'm not sure what we should do here: define it only if not yet defined, so that we trust the system to do what's right? Or call it __openlibm_pure?

@ViralBShah
Copy link
Member

I would prefer to do the conditional define. Changing these things more means more difficulty in importing upstream changes.

@ViralBShah
Copy link
Member

The failing cases are all 1 ulp and some FP exception stuff. I would say that if no new failures arise, we should be ok.

@ViralBShah
Copy link
Member

If we can address the __pure2 stuff, let's get this in for the 0.5 release. Nice to have but not essential.

@nalimilan
Copy link
Contributor Author

I'm preparing a fix.

This gets rid of a warning on Mac OS X due to the fact that
sys/cdefs.h defines it already.
@nalimilan
Copy link
Contributor Author

With this small patch it should be good to go. I agree the errors can wait for another release since they aren't regressions.

ViralBShah added a commit that referenced this pull request Mar 8, 2016
@ViralBShah ViralBShah merged commit 4fcad15 into master Mar 8, 2016
@ViralBShah ViralBShah deleted the nl/includes2 branch March 8, 2016 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants