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

Add align_offset intrinsic #43903

Merged
merged 2 commits into from
Aug 30, 2017
Merged

Add align_offset intrinsic #43903

merged 2 commits into from
Aug 30, 2017

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Aug 16, 2017

see rust-lang/rfcs#2043 for details and the plan towards stabilization (reexport in core::mem via various convenience functions)

as per @scottmcm 's comment, this is just the intrinsic (which is obviously unstable).

@rust-highfive
Copy link
Collaborator

r? @aturon

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

@aidanhs aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 16, 2017
/// overflow or go beyond the allocation that `ptr` points into.
/// It is up to the caller to ensure that the returned offset is correct
/// in all terms other than alignment.
pub fn align_offset(ptr: *const (), align: usize) -> usize {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Should this be unsafe, for consistency with the real intrinsic?

/// There are no guarantees whatsover that offsetting the pointer will not
/// overflow or go beyond the allocation that `ptr` points into.
/// It is up to the caller to ensure that the returned offset is correct
/// in all terms other than alignment.
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding an # Examples doctest to demo a few simple cases.

/// # use std::mem::align_of;
/// let x = [5u8, 6u8, 7u8, 8u8, 9u8];
/// let ptr = &x[n] as *const u8;
/// let offset = align_offset(ptr as *const (), align_of::<u16>());
Copy link
Member

Choose a reason for hiding this comment

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

doc test failed.

[00:56:48] failures:
[00:56:48] 
[00:56:48] ---- intrinsics.rs - intrinsics::align_offset (line 1361) stdout ----
[00:56:48] 	error: use of unstable library feature 'core_intrinsics': intrinsics are unlikely to ever be stabilized, instead they should be used through stabilized interfaces in the rest of the standard library (see issue #0)
[00:56:48]  --> intrinsics.rs:9:14
[00:56:48]   |
[00:56:48] 9 | let offset = align_offset(ptr as *const (), align_of::<u16>());
[00:56:48]   |              ^^^^^^^^^^^^
[00:56:48]   |
[00:56:48]   = help: add #![feature(core_intrinsics)] to the crate attributes to enable
[00:56:48] 
[00:56:48] error: use of unstable library feature 'core_intrinsics': intrinsics are unlikely to ever be stabilized, instead they should be used through stabilized interfaces in the rest of the standard library (see issue #0)
[00:56:48]  --> intrinsics.rs:5:5
[00:56:48]   |
[00:56:48] 5 | use std::intrinsics::align_offset;
[00:56:48]   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[00:56:48]   |
[00:56:48]   = help: add #![feature(core_intrinsics)] to the crate attributes to enable
[00:56:48] 
[00:56:48] error[E0308]: mismatched types
[00:56:48]   --> intrinsics.rs:11:30
[00:56:48]    |
[00:56:48] 11 |     let u16_ptr = ptr.offset(offset) as *const u16;
[00:56:48]    |                              ^^^^^^ expected isize, found usize
[00:56:48] 
[00:56:48] thread 'rustc' panicked at 'couldn't compile the test', /checkout/src/librustdoc/test.rs:280:12
[00:56:48] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:56:48] 
[00:56:48] 
[00:56:48] failures:
[00:56:48]     intrinsics.rs - intrinsics::align_offset (line 1361)
[00:56:48] 
[00:56:48] test result: FAILED. 1231 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out
[00:56:48] 
[00:56:48] error: test failed, to rerun pass '--doc'

Verified

This commit was signed with the committer’s verified signature.
see rust-lang/rfcs#2043 for details

Verified

This commit was signed with the committer’s verified signature.
@oli-obk
Copy link
Contributor Author

oli-obk commented Aug 28, 2017

ping @aturon

@aturon
Copy link
Member

aturon commented Aug 30, 2017

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 30, 2017

📌 Commit be96ad2 has been approved by aturon

@bors
Copy link
Contributor

bors commented Aug 30, 2017

⌛ Testing commit be96ad2 with merge c66e7fa...

bors added a commit that referenced this pull request Aug 30, 2017

Verified

This commit was signed with the committer’s verified signature.
Add align_offset intrinsic

see rust-lang/rfcs#2043 for details and the plan towards stabilization (reexport in `core::mem` via various convenience functions)

as per @scottmcm 's [comment](rust-lang/rfcs#2043 (comment)), this is just the intrinsic (which is obviously unstable).
@bors
Copy link
Contributor

bors commented Aug 30, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing c66e7fa to master...

@bors bors merged commit be96ad2 into rust-lang:master Aug 30, 2017
@oli-obk oli-obk deleted the alignto branch August 30, 2017 11:01
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Dec 18, 2017

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…trochenkov

Fix the wrong subtraction in align_offset intrinsic.

Given how the stage0 implementation in rust-lang#43903 is written, as well as that in the RFC, I suppose the current implementation has a typo.

cc rust-lang#44488, cc @oli-obk.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants