Opened 17 months ago

Closed 4 months ago

#25727 closed enhancement (wontfix)

Add bool types to Rust coding standards guidelines

Reported by: isis Owned by: catalyst
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: torspec, rust, fast-fix, 035-triaged-in-20180711
Cc: Actual Points:
Parent ID: Points: .1
Reviewer: catalyst Sponsor: SponsorM-can

Description

Similar to #25368, we can once again expand upon which Rust types are safe to send over the FFI boundary without conversion/serialisation/translation, because Rust's bool type and C99's bool type are directly compatible. (I think this also makes an even larger argument moving forward for using bools in C wherever it makes sense, since otherwise we need to convert between the libc uint8_t type and the native u8 in Rust.)

Child Tickets

Change History (10)

comment:1 Changed 17 months ago by isis

Status: assignedneeds_review

Done in my bug25727 branch.

comment:2 Changed 17 months ago by dgoulet

Reviewer: catalyst

comment:3 Changed 16 months ago by catalyst

Status: needs_reviewneeds_revision

Summarizing yesterday's IRC discussion:

  • Rust has debated whether Rust's bool is identical to C99's _Bool for a while.
  • Rust now specifies that its bool is one byte.
  • There is a postponed RFC to define Rust's bool and C99's _Bool to be FFI-compatible.

We should either postpone merging this until Rust decides to accept RFC 954 or point to that RFC (and maybe the follow up RFC 992) in our doc. There seems to be a consensus that it should be accepted but it seems they haven't gone through the formalities yet.

This patch also adds a non-ASCII character in a file that previously had none. I think we lack a style guide that covers the acceptability of non-ASCII in various files, but this is a change. On the other hand CodingStandardsRust.md already had some non-ASCII characters in it so maybe we don't actually care that much?

comment:4 Changed 16 months ago by isis

I've left a comment on Rust RFC 992 here.

comment:5 Changed 16 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: 0.3.5.x-final

Move most needs_revision tickets from 0.3.4 to 0.3.5: we're about to hit the feature freeze.

If you really need to get this into 0.3.4, please drop me a line. :)

comment:6 Changed 14 months ago by nickm

Keywords: 035-triaged-in-20180711 added

comment:7 Changed 13 months ago by catalyst

I left a comment on the RFC here back in May but haven't received a reply yet.

comment:8 Changed 12 months ago by nickm

Milestone: Tor: 0.3.5.x-finalTor: unspecified

We won't get PQ or wide-create done in this release. :/

comment:9 Changed 4 months ago by catalyst

Owner: changed from isis to catalyst
Status: needs_revisionassigned

comment:10 Changed 4 months ago by catalyst

Resolution: wontfix
Status: assignedclosed

Unfortunately, it seems like Rust RFC 992 was closed without taking the final step of explicitly documenting the ABI equivalence of Rust's bool and C's _Bool. Further comments from a Rust core team member imply that this is unlikely to be resolved any time soon.

Note: See TracTickets for help on using tickets.