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

Reuse sys::unix::cmath on other platforms #84522

Merged
merged 1 commit into from
Apr 30, 2021
Merged

Conversation

CDirkx
Copy link
Contributor

@CDirkx CDirkx commented Apr 24, 2021

Reuse sys::unix::cmath on all non-windows platforms.

unix is chosen as the canonical location instead of unsupported or common because unsupported doesn't make sense semantically and common is reserved for code that is supported on all platforms. Also unix is already the home of some non-windows code that is technically not exclusive to unix like unix::path.

@rust-highfive
Copy link
Collaborator

r? @yaahc

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 24, 2021
@@ -1,32 +1,31 @@
#![cfg(not(test))]

use libc::{c_double, c_float};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These functions are used to implement methods of f32 and f64, so anything else then c_double = f64 and c_float = f32 would be wrong anyway. This makes the file reusable on platforms that don't have libc available.

@CDirkx

This comment has been minimized.

@rustbot rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Apr 24, 2021
@CDirkx CDirkx mentioned this pull request Apr 24, 2021
@yaahc
Copy link
Member

yaahc commented Apr 26, 2021

Everything here looks good to me but I'm not super familiar with the platform abstraction modules yet so I'd like to get a second set of eyes on this before approving.

cc @rust-lang/libs

@m-ou-se m-ou-se self-assigned this Apr 26, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Apr 26, 2021

(Adding myself to the assignees so I won't forget to look at it tomorrow.)

@m-ou-se
Copy link
Member

m-ou-se commented Apr 27, 2021

Looks good to me, other than the above comment.

@yaahc
Copy link
Member

yaahc commented Apr 29, 2021

Looks like it's good to go then

@bors r+

@bors
Copy link
Contributor

bors commented Apr 29, 2021

📌 Commit 26fb1e3 has been approved by yaahc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 30, 2021
Reuse `sys::unix::cmath` on other platforms

Reuse `sys::unix::cmath` on all non-`windows` platforms.

`unix` is chosen as the canonical location instead of `unsupported` or `common` because `unsupported` doesn't make sense semantically and `common` is reserved for code that is supported on all platforms. Also `unix` is already the home of some non-`windows` code that is technically not exclusive to `unix` like `unix::path`.
@bors
Copy link
Contributor

bors commented Apr 30, 2021

⌛ Testing commit 26fb1e3 with merge 49920bc...

@bors
Copy link
Contributor

bors commented Apr 30, 2021

☀️ Test successful - checks-actions
Approved by: yaahc
Pushing 49920bc to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 30, 2021
@bors bors merged commit 49920bc into rust-lang:master Apr 30, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 30, 2021
@CDirkx CDirkx deleted the cmath branch April 30, 2021 12:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants