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 a type which is equivalent to _Bool in C99 #992

Closed
alexcrichton opened this issue Mar 19, 2015 · 16 comments
Closed

Add a type which is equivalent to _Bool in C99 #992

alexcrichton opened this issue Mar 19, 2015 · 16 comments
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.

Comments

@alexcrichton
Copy link
Member

Postponed RFC

This type would have to ensure two aspects:

  1. The size of the representation is the same as the backing compiler's representation.
  2. Each value is only ever allowed to contain the integer 0 or 1.

This could possibly be done through a new c_bool primitive type in liblibc (not an alias) or perhaps a wrapper struct/wrapper newtype if we're able to enhance as semantics enough (for example).

@Timmmm
Copy link

Timmmm commented Dec 2, 2016

Did this ever happen? What should I do if I'm wrapping a C library that uses bool (_Bool)?

Edit: Never mind; seems it is still in the works but according to this comment all compilers/systems that you'd care about make _Bool a single byte, which is compatible with Rust's bool.

@strega-nil
Copy link

I would argue that we should define Rust's bool to be equivalent to _Bool.

@Yamakaky
Copy link

Yamakaky commented Dec 2, 2016

Wouldn't it prevent the compiler to do things like pack 8 bool in a byte? Or does repr(C) would take care of that?

@eddyb
Copy link
Member

eddyb commented Dec 3, 2016

@Yamakaky Packing like that would have to be opt-in because you can take references to any fields.

@Timmmm
Copy link

Timmmm commented Dec 3, 2016

Well, I'm not a rust pro, but I'd be happy with a c_bool that is defined to match _Bool and lets you convert between Rust's native bool and c_bool using as. That way you don't need to have Rust's native bool follow some weird C 'an integer at least as big as blah' stuff.

@Amanieu
Copy link
Member

Amanieu commented Dec 3, 2016

There are basically 2 ways to define c_bool:

  • The easy way: type c_bool = bool
  • The hard way:
#[repr(i8)]
enum c_bool {
    c_false = 0,
    c_true = 1,
}

The first one is easier to use, but ties our representation of bool with that of _Bool. Both use the same underlying representation (1 byte which is either 0 or 1) for all supported platforms, but in theory (though very unlikely) a future platform may appear which uses a different representation for _Bool.

@strega-nil
Copy link

strega-nil commented Dec 4, 2016

@Amanieu I think that tying our representation of bool to _Bool is better; too many people have written extern functions assuming that bool is equivalent, that to do otherwise would be a serious back-compat concern, but only on esoteric platforms where one may not have tested their code.

gnomesysadmins pushed a commit to GNOME/librsvg that referenced this issue Apr 26, 2017
See rust-lang/rfcs#992 - bool in Rust does not
repr(C) as any C type in particular.  I was naively assuming that it
would repr(C) as int, which is equivalent to gboolean.

This makes tests/fixtures/dimensions/bug612951.svg pass again.  We were
calling rsvg_node_svg_get_view_box() and getting a garbage value inside
vbox.active - the low byte actually set as expected, and the high bits
set to garbage.
@le-jzr
Copy link

le-jzr commented Apr 27, 2017

I feel like this issue needs to be resolved.

Currently there is no stable way to use _Bool in FFI, and as long as Rust bool and _Bool compatible by accident, indecision just means defaulting to @ubsan's solution (eventually, de facto use of bool for the purpose will prevent any changes to its representation without silently breaking all that code).

@strega-nil
Copy link

@le-jzr it's already de facto resolved (and basically de jure resolved). There's no way to change it without silently breaking a ton of code.

@Amanieu
Copy link
Member

Amanieu commented Apr 28, 2017

It's just a matter of adding type c_bool = bool to the libc crate to confirm it.

@le-jzr
Copy link

le-jzr commented Apr 28, 2017

@ubsan According to #954 (comment), specifying that bool is _Bool has not been the intention. While that position might have changed, I haven't found any comments indicating so.

Note carefully that I'm not saying I'm categorically against that. I'm just saying it hasn't been stabilized yet. That's the principal problem. As long as it's unstable, then de jure that "ton of code" you speak of is a ton of undefined behavior, even though it de facto works right now.

That said, according to #954 (comment), the current representation of bool matches _Bool on all supported platforms, so we should in fact have type c_bool = bool in either case. That's a separate thing from defining them equal. FFI code using bool directly would still be invalid until and unless #954 passes, since bool could change representation while c_bool just switches definition to compensate.

@strega-nil
Copy link

@le-jzr but it's also allowed in FFI functions without warning (which is close to de jure confirmation).

est31 added a commit to est31/rust that referenced this issue Nov 22, 2017
The ABI of bool is reserved, see these links:

* rust-lang/rfcs#954 (comment)
* rust-lang#46156 (comment)
* rust-lang/rfcs#992

Currently the improper_ctypes lint is inconsistent
with that position by treating bools as if they had
a specified ABI.

To fix this inconsistency, this changes treatment of
bools by the lint.

This might break some code, so possibly it has to
be phased in slowly...
@petrochenkov petrochenkov added T-libs-api Relevant to the library API team, which will review and decide on the RFC. and removed A-libs labels Jan 29, 2018
@isislovecruft
Copy link

isislovecruft commented Apr 23, 2018

Hi! At @torproject, we recently decided for tor that it's permissible to write C99, and we now have a function returning a bool that I'd like to wrap in an FFI definition for exposure to our Rust code (cf. https://bugs.torproject.org/25727).

We've noted that bool should be compatible to C/++'s _Bool, however a theoretical-but-currently-non-existent compiler could create a multi-byte _Bool type. However, it appears that Rust's bool has now been defined as a single byte, so the theoretically-weird C compiler edge case is out of the picture.

On the other hand, it seems the improper_ctypes lint was recently modified to add bool. It'd be really nice to have some closure on whether or not bools can cross the FFI boundary, since it might affect our C coding standards moving forward.

If the ABI of bool is decided to remain reserved, would it be acceptible to define a c_bool type in the libc crate? I'd be happy to write a patch.

Is it possible to come to a decision on this issue soon?

@strega-nil
Copy link

@isislovecruft I believe that the RFC was closed, and that the representation of bool will be the same as _Bool on all platforms? I may be wrong.

@tlyu
Copy link

tlyu commented May 15, 2018

It looks like @withoutboats proposed basically accepting #954 (at rust-lang/rust#46156 (comment)) and also thinks that Rust's bool should be documented as having the same representation as C's _Bool (rust-lang/rust#46176 (comment)). There was some amount of agreement, and no obvious significant objections.

I'm not very familiar with the Rust RFC process, so I'm not sure what needs to happen next if we want this to move forward. Does someone need to reopen #954? Or should someone propose a FCP on this RFC? Or something else?

@Centril
Copy link
Contributor

Centril commented Oct 7, 2018

It seems to me that given rust-lang/rust#46156 (comment) and other changes noted in #992 (comment) the points in OP are completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests