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.
Requirements on the target platform C implementation #46
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
Requirements on the target platform C implementation #46
Changes from 8 commits
36fd33e
7fc7f85
df2e236
8e009a1
07f7021
aadac80
2e0d82f
fcfb9c8
612f003
f89431d
25097ac
36d3fb3
c508026
e9dbb11
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 think Rust should be able to target these platforms but Rust programs can't reference any object at address zero.
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.
Regardless of the value of
NULL
(zero or otherwise), it should be straightforward for the Rust compiler to translate between a pointer andOption<&T>
if the definition ofstd::ptr::NonNull
were made more flexible; currently its documentation conflates zero andNULL
..OTOH, I think that would be a breaking change so I think it would be OK to restrict the current edition of Rust to platforms where NULL is zero since there's probably a lot of things in Rust that assume that already.
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 documentation for
NonNull
says "*mut T but non-zero and covariant" (emphasis mine). Later on the documentation says "non-null"; i.e."non-zero" and "non-null" are used interchangeably.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 docs might need fixing.
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 agree. However, somebody may have already read the documentation and made the assumption that NULL == 0 based on them, and may then be relying on that, so it should be announced loudly if that change is made. If it isn't too late, it would be good to do it at the same time Rust 2018 edition ships and maybe consider it part of Rust 2018.
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 is unclear at this point what we are going to require about the niches of
&T
, that probably will be part of the discussions about "validity", which are not taking place yet (hence why this is an unresolved question).