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

Is it possible to map usize to size_t and isize to ptrdiff_t? #239

Closed
lu-zero opened this issue Oct 31, 2018 · 10 comments
Closed

Is it possible to map usize to size_t and isize to ptrdiff_t? #239

lu-zero opened this issue Oct 31, 2018 · 10 comments

Comments

@lu-zero
Copy link
Contributor

lu-zero commented Oct 31, 2018

I'm not sure if there is a mean to annotate to do this mapping.

@RReverser
Copy link
Contributor

Why do you want this? Changing usize to size_t would be incorrect and might break on certain platforms.

@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 31, 2018

Which platforms exactly?

IIRC uintptr is an optional type and could be missing or being different from the register size (e.g. x32 and mips hybrids)

@RReverser
Copy link
Contributor

RReverser commented Oct 31, 2018

You might want to check answers to same question previously asked in #198. Rust's usize is explicitly defined as pointer-wide unsigned integer, which maps only to uintptr_t, but C and C++ define size_t differently - they are intended for holding any array indices, which might not necessarily have the same size as pointer, e.g. on weird platforms with segmented memory.

@lu-zero
Copy link
Contributor Author

lu-zero commented Oct 31, 2018

The way we use usize to hold len() and size_of() values, makes it a little more confusing.

I'm not even sure rust itself supports architectures with sizeof(size_t) != sizeof(uintptr_t) and sizeof(ptrdiff_t) != sizeof(intptr_t).

Given that intptr_t is optional and makes C people complain because looks weird to them, I'd rather map it to size_t as long the two are compatible, thus why I'd rather have a way to annotate the type so it maps to what the user expects (size_t for array lengths and ptrdiff_t for strides).

The defaults can stay as they are :)

@RReverser
Copy link
Contributor

RReverser commented Nov 1, 2018

I'm not even sure rust itself supports architectures with sizeof(size_t) != sizeof(uintptr_t) and sizeof(ptrdiff_t) != sizeof(intptr_t).

I think it does, it just doesn't have equivalent of size_t and instead uses pointer-sized integers for everything, as you noticed. Nevertheless, when translating to C/C++, it's important to follow their standards, not Rust ones, to be actually safely compatible across all possible platforms where Rust might be used via FFI bindings.

Given that intptr_t is optional and makes C people complain because looks weird to them

That sounds like a strange personal preference, not something to base choice of type representation on, especially because

as long the two are compatible

is not actually guaranteed at all and causes user to rely on UB, which can go really wrong.

@lu-zero
Copy link
Contributor Author

lu-zero commented Nov 1, 2018

My problem is getting the users what they expect, I have no problems in adding a build/configure time checks to make sure the incompatible platforms are managed and then use the right type and then annotate it so it is expressed as they want.

@lu-zero
Copy link
Contributor Author

lu-zero commented Nov 1, 2018

To expand, I'd be glad to have c_size_t and c_ptrdiff_t on the rust side and have it emitted to the C side.

@lu-zero
Copy link
Contributor Author

lu-zero commented Nov 1, 2018

Actually libc provides both, but cbindgen is recognizing only size_t as a primitive type. PR sent.

@eqrion
Copy link
Collaborator

eqrion commented Nov 5, 2018

To expand, I'd be glad to have c_size_t and c_ptrdiff_t on the rust side and have it emitted to the C side.

Yes, the preferred solution here would be for the rust side to use the native types that C/C++ expect. As @RReverser notes, usize and isize are not equivalent to size_t or ptrdiff_t. Adding a configuration option here would be too big of a footgun and maintenance burden in my opinion.

@eqrion
Copy link
Collaborator

eqrion commented Nov 5, 2018

I merged PR #242 to resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants