Skip to content

Define c_char using cfg_if rather than repeating 40-line cfg #91641

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Jan 28, 2022

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Dec 7, 2021

Libstd has a 40-line cfg that defines the targets on which c_char is unsigned, and then repeats the same cfg with not(…) for the targets on which c_char is signed.

This PR replaces it with a cfg_if! in which an else takes care of the signed case.

I confirmed that x.py doc library/std inlines the type alias because c_char_definition is not a publicly accessible path:

Screenshot from 2021-12-07 13-42-07

@rust-highfive
Copy link
Contributor

r? @kennytm

(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 Dec 7, 2021
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 7, 2021

📌 Commit db5a2ae has been approved by Mark-Simulacrum

@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 Dec 7, 2021
@camelid camelid added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Dec 8, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
Define c_char using cfg_if rather than repeating 40-line cfg

Libstd has a 40-line cfg that defines the targets on which `c_char` is unsigned, and then repeats the same cfg with `not(…)` for the targets on which `c_char` is signed.

This PR replaces it with a `cfg_if!` in which an `else` takes care of the signed case.

I confirmed that `x.py doc library/std` inlines the type alias because c_char_definition is not a publicly accessible path:

![Screenshot from 2021-12-07 13-42-07](https://user-images.githubusercontent.com/1940490/145110596-f1058406-9f32-44ff-9a81-1dfd19b4a24f.png)
@fee1-dead
Copy link
Member

Looks like this caused a clippy test to fail in a rollup: #91650 (comment). The stderr now tells users to remove cast to c_char which would actually break compilation for some platforms.

@dtolnay
Copy link
Member Author

dtolnay commented Dec 8, 2021

I filed rust-lang/rust-clippy#8093 about the false positive and added a workaround in this PR. Tested by:

//#[cfg(all())]  // unnecessary_cast goes away if uncommented
type T = u8;

fn main() {
    let _ = 0 as T;
}

@dtolnay dtolnay added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 8, 2021
@dtolnay dtolnay assigned Mark-Simulacrum and unassigned kennytm Jan 27, 2022
@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 27, 2022

📌 Commit 4e8b91a has been approved by Mark-Simulacrum

@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 Jan 27, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 27, 2022
…askrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#91641 (Define c_char using cfg_if rather than repeating 40-line cfg)
 - rust-lang#92899 (Mention std::iter::zip in Iterator::zip docs)
 - rust-lang#93193 (Add test for stable hash uniqueness of adjacent field values)
 - rust-lang#93325 (Introduce a limit to Levenshtein distance computation)
 - rust-lang#93339 (rustdoc: add test case for multiple traits and erased names)
 - rust-lang#93357 (Clarify the `usage-of-qualified-ty` error message.)
 - rust-lang#93363 (`#[rustc_pass_by_value]` cleanup)
 - rust-lang#93365 (More arena cleanups)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4af3930 into rust-lang:master Jan 28, 2022
@rustbot rustbot added this to the 1.60.0 milestone Jan 28, 2022
@dtolnay dtolnay deleted the cchar-if branch January 31, 2022 17:55
@dtolnay dtolnay added A-FFI Area: Foreign function interface (FFI) and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-FFI Area: Foreign function interface (FFI) 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.

8 participants