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

rustc: Don't lint about isize/usize in FFI #28779

Merged
merged 1 commit into from
Oct 6, 2015

Conversation

alexcrichton
Copy link
Member

This lint warning was originally intended to help against misuse of the old Rust
int and uint types in FFI bindings where the Rust int was not equal to the
C int. This confusion no longer exists (as Rust's types are now isize and
usize), and as a result the need for this lint has become much less over time.

Additionally, starting with the RFC for libc it's likely that isize and
usize will be quite common in FFI bindings (e.g. they're the definition of
size_t and ssize_t on many platforms).

This commit disables these lints to instead consider isize and usize valid
types to have in FFI signatures.

This lint warning was originally intended to help against misuse of the old Rust
`int` and `uint` types in FFI bindings where the Rust `int` was not equal to the
C `int`. This confusion no longer exists (as Rust's types are now `isize` and
`usize`), and as a result the need for this lint has become much less over time.

Additionally, starting with [the RFC for libc][rfc] it's likely that `isize` and
`usize` will be quite common in FFI bindings (e.g. they're the definition of
`size_t` and `ssize_t` on many platforms).

[rfc]: rust-lang/rfcs#1291

This commit disables these lints to instead consider `isize` and `usize` valid
types to have in FFI signatures.
@rust-highfive
Copy link
Collaborator

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @nrc

@rust-highfive rust-highfive assigned nrc and unassigned pcwalton Sep 30, 2015
@alexcrichton
Copy link
Member Author

cc @briansmith, #28096

@briansmith
Copy link
Contributor

Great. Although I'd figured out how to change the lint implementation easy enough, I was struggling to figure out how to update the tests. Thanks for taking this over.

@alexcrichton
Copy link
Member Author

No problem! Thanks again for proposing the change :)

@bluss bluss added the relnotes Marks issues that should be documented in the release notes of the next release. label Oct 1, 2015
@nrc
Copy link
Member

nrc commented Oct 5, 2015

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 5, 2015

📌 Commit bd6758a has been approved by nrc

bors added a commit that referenced this pull request Oct 6, 2015
This lint warning was originally intended to help against misuse of the old Rust
`int` and `uint` types in FFI bindings where the Rust `int` was not equal to the
C `int`. This confusion no longer exists (as Rust's types are now `isize` and
`usize`), and as a result the need for this lint has become much less over time.

Additionally, starting with [the RFC for libc][rfc] it's likely that `isize` and
`usize` will be quite common in FFI bindings (e.g. they're the definition of
`size_t` and `ssize_t` on many platforms).

[rfc]: rust-lang/rfcs#1291

This commit disables these lints to instead consider `isize` and `usize` valid
types to have in FFI signatures.
@bors
Copy link
Contributor

bors commented Oct 6, 2015

⌛ Testing commit bd6758a with merge 88b6a50...

@bors bors merged commit bd6758a into rust-lang:master Oct 6, 2015
@alexcrichton
Copy link
Member Author

Nominating for a beta backport, I think this will be an instrumental part of rust-lang/rfcs#1291 and I think the impact is pretty low.

@alexcrichton alexcrichton deleted the ffi-isize-usize branch October 6, 2015 05:56
@alexcrichton alexcrichton added beta-nominated Nominated for backporting to the compiler in the beta channel. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Oct 6, 2015
@pnkfelix
Copy link
Member

@rust-lang/lang I have no opinion about backporting this to beta; I certainly don't object, but I also don't think its the end of the world if we end up waiting a release cycle for this change to the lint.

@nikomatsakis
Copy link
Contributor

I wouldn't normally want to backport this, but the patch is small, and so if @alexcrichton is in favor I'm ok with it.

@nikomatsakis nikomatsakis added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 16, 2015
@brson brson mentioned this pull request Oct 16, 2015
@alexcrichton alexcrichton removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. relnotes Marks issues that should be documented in the release notes of the next release. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants