Lint for the reserved ABI of bool

[est31]

The ABI of bool is reserved, see these links:

• Specify that bool is compatible with the _Bool C type. rfcs#954 (comment)
• Document the size of bool #46156 (comment)
• Add a type which is equivalent to _Bool in C99 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...

[rust-highfive]

r? @michaelwoerister

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

[est31]

I don't neccessarily endorse this PR myself. If the apparent inconsistency gets fixed by having bool finally specified, I would be happier.

[hsivonen]

As someone who has (successfully, AFAICT, on multiple platforms—the ones Firefox 56+ runs on) used bool in FFI with the assumption that it has the same representation as _Bool, I'd much prefer Rust to officially guarantee that instead of saying that booleans aren't a legitimate part of FFI. In other words, I think this shouldn't be merged.

[michaelwoerister]

Nominating for discussion in the @rust-lang/compiler team meeting.

[hsivonen]

To elaborate: Not having a type that is equivalent to _Bool in FFI is terribly inconvenient in terms of not being able to expose an idiomatic C11 API for a Rust library directly but having to expose a legacy C API and wrap that as a C11 API on the C side. But worse, not having _Bool in FFI means that Rust can't call C11 libraries without a C-side wrapper.

As for Rust bool being the type that's equivalent to _Bool in FFI, that's already the case in practice (AFAICT), so making it not so is a needless inconvenience.

As for weird embedded C compilers having weird _Bool, I think the inconvenience should be on those who wish to link with code from such compilers instead of everyone working on normal (GCC, clang, MSVC) C compilers being inconvenienced. Rust already can't interop with all imaginable unreasonable but C-compliant C implementations due to Rust relying on two's complement representation of signed integers. Likewise, it should be OK for Rust to interop with _Bool for mainstream C only.

[steveklabnik]

New lints require an RFC. This kind of blurs the line.

[est31]

This is more a bugfix than a new lint addition or something like that ("feature change" etc). Unless I understood it wrongly, the purpose of the improper_ctypes lint is to check for types that have no defined ABI. Right now it seems to have missed the bool type!

[hsivonen]

When bool works in FFI in practice today on stable channel, making it not work or making the compiler complain about it would break Rust's stability promise in practice. (Even if a working stable feature works unintentionally, making it not work or making the compiler complain about it is a breaking change: Rust programmers should be able to rely on the stability of stable Rust without researching which functionality works intentionally and which funtionality works unintentionally. Additionally, I think the existing behavior of bool working in FFI with mainstream C implementations is desirable behavior.)

[est31]

it would break Rust's stability promise in practice

It is established practice that something can be broken if it only works due to a bug. See this search. And the current behaviour is clearly a bug: the ABI of bool is not specified yet, even though the lint assumes that it is specified.

Probably this change has to be phased in slowly, as the other breaking changes, I'll change my PR if the compiler team has deemed that they want to do the change.

[bluss]

What you are saying is that this is not credible as a soundness bug fix, even of a lint. I understand the not credible part. On the other hand, ffi is unsafe code zone and there are many details the compiler can not check for you there.

It should only be good if we get better at this approximate and pragmatic safety work of warning about portability and safety issues in code outside of safe rust.

[hsivonen]

There's a big difference between a theoretical break to fix a soundness bug and withdrawing a practically working behavior that's in use in deployed code over a theoretical concern.

As for FFI being unsafe, the use of a particular type in a signature is not the kind of unsafe that the compiler couldn't have checked, so I don't think FFI being unsafe makes it OK to make a breaking change.

Withdrawing a working feature because it was not specified officially seems like the C approach of blaming the programmer for relying on what wasn't specified as reliable and not like Rust's stability story. ☹️

[hsivonen]

As for it being just a lint: Either the lint is the first step towards breaking it, in which case it's not just a lint, or the existing stable behavior is going to continue to work in which case the lint would uselessly bother programmers on mainstream platforms.

A lint that warns all programmers that their FFI code might not link with C code produced on some niche platform that Rust doesn't support yet would be a gross misbalancing of presently relevant mainstream needs vs. potential future niche concerns.

[est31]

@hsivonen this PR is only enacting the existing policy on this issue, which is that bool has an unspecified ABI. I think you should better talk to the lang team to get the policy specified. This would be the best outcome, and then this PR can be closed.

[hsivonen]

What's the right place to discuss the policy?

[hsivonen]

What's the right place to discuss the policy?

I posted to the internals forum.

[est31]

@hsivonen thanks for the post!

[arielb1]

This is being discussed in the internals thread and needs more research.

[rfcbot]

Team member @withoutboats has proposed to close this. The next step is review by the rest of the tagged teams:

• @Zoxc
• @arielb1
• @aturon
• @cramertj
• @eddyb
• @estebank
• @jseyfried
• @michaelwoerister
• @nikomatsakis
• @nrc
• @petrochenkov
• @pnkfelix
• @withoutboats

No concerns currently listed.

Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[hanna-kruppe]

Why that way around and not the alternative? (Specify how bool currently behaves, and document that this matches _Bool on all platforms we support)

[withoutboats]

Why that way around and not the alternative? (Specify how bool currently behaves, and document that this matches _Bool on all platforms we support)

People could come to the conclusion that they need a c_bool type for their FFI to be forward compatible with platforms we don't yet support. I think defining it as the same representation as _Bool / C++ bool makes it the least likely someone does something painful to avoid entirely hypothetical problems.

[est31]

@withoutboats do I understand you correctly that this "close" FCP also functions as a "merge" FCP for #46156 ?

[withoutboats]

I think we should merge that PR and also somewhere (the reference?) clarify precisely that it has the same representation as _Bool.

[nikomatsakis]

@withoutboats agreed; I was going to propose the same.

@cuviper thanks, just what I wanted to know

[est31]

friendly ping @jseyfried , your mark is missing.

[nikomatsakis]

I took the libery of tagging for @jseyfried

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[est31]

Closing in favour of #46156

[luke-jr]

Just to clarify: do we know of any cases where C++ bool and C's _Bool are treated differently?

Just to answer a few years later: It's more of a detail of C++'s std::vector than bool, but std::vector<bool> only uses one bit per bool (though this is optional in the standard).