-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Specify that bool
is compatible with the _Bool
C type.
#954
Closed
Closed
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
- Feature Name: cbool | ||
- Start Date: Mon Mar 9 00:16:53 CET 2015 | ||
- RFC PR: (leave this empty) | ||
- Rust Issue: (leave this empty) | ||
|
||
# Summary | ||
|
||
Specify that `bool` is compatible with the `_Bool` C type. | ||
|
||
# Motivation | ||
|
||
You cannot safely call ffi functions with boolean arguments without a compatible type. | ||
|
||
# Detailed design | ||
|
||
Specify that `bool` is compatible with the `_Bool` C type and inform the compiler that `bool` is ffi safe. | ||
|
||
# Drawbacks | ||
|
||
None right now. This should be a no-op on all supported platforms. | ||
|
||
# Alternatives | ||
|
||
Define `_Bool` as a platform dependent integer type. This is unsafe because the behavior is supposedly undefined if you pass a value other than 0 or 1 to such a function. | ||
|
||
# Unresolved questions | ||
|
||
None. |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about arbitrary unsupported platforms? This proposal has the ability to box us into a suboptimal corner, by being beholden to whatever quirks C might have.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsupported platforms are not supported, hence we're not beholden to do anything on those platforms, unless I misunderstand your concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are only unsupported now. If/when we gain support for them we will have to guarantee that
bool
acts like_Bool
on those platforms too, so we need to plan ahead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It seems unlikely to me that there will be any platforms where
_Bool
is different than ourbool
(especially considering that we will only ever target platforms LLVM supports, which means there is some degree of sanity), but I guess it is possible.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the plan for platforms that don't use two's complement? If there is no such plan, then why would you worry more about the representation of
bool
than the representation ofu8
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Also, I have no idea if
_Bool
is one byte on all supported platforms, sobool
still may not match_Bool
even with that property.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The premise of this argument is pretty ridiculous: "We know better than the hardware developers how to represent a boolean value on their hardware."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not ridiculous: the C
_Bool
type on some platforms may have historical constraints that don't apply to Rust. In any case, the precise representation of C types is usually chosen by the compiler, not the hardware.A minimum improvement that would help assuage my concerns would be quoting the C standard's definition of
_Bool
in the RFC.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's wrong. The representation of _Bool is part of the ABI and should be (for all hardware developed after C99) defined by the hardware vendor. E.g.
http://www.x86-64.org/documentation/abi.pdf page 12
http://infocenter.arm.com/help/topic/com.arm.doc.ihi0042e/IHI0042E_aapcs.pdf page 26
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That information should be in the RFC.