core: Express -1 as ~0 in thread_status_t cast

[SimonIT]

When compiling rust-hello-world for the nrf52840dongle, c2rust seem to generate code which casts -1 to u8 which fails because it's unsigned. Changing to ~0 fixes this.

error[E0600]: cannot apply unary operator `-` to type `u8`
    --> RIOT/examples/rust-hello-world/bin/nrf52840dongle/target/thumbv7em-none-eabihf/release/build/riot-sys-a758a33386ee540f/out/riot_c2rust_replaced.rs:6813:39
     |
6813 |     let mut result: thread_status_t = -1 as thread_status_t;
     |                                       ^^
     |                                       |
     |                                       cannot apply unary operator `-`
     |                                       help: you may have meant the maximum value of `u8`: `u8::MAX`
     |
     = note: unsigned values cannot be negated

For more information about this error, try `rustc --explain E0600`.
error: could not compile `riot-sys` (lib) due to previous error

Contribution description

Change -1 to ~0 for casting to u8

Testing procedure

Issues/PRs references

make BOARD=nrf52840donglein rust-hello-world

[chrysn]

Tempting as it would be to start asking different C compilers for their interpretation of the relevant rules on whether the cast is OK, let's be pragmatic. (That also extends to drilling down how that worked earlier and doesn't now: I'm doing a bit of that, but there is no outcome that'd make this patch less valid).

Thanks for the fix, ACK.

[chrysn]

As it's probably hard to spot: The CI is unhappy with your commit message, as it says "fix:" (which it interprets like git's "fixup!" convention); a coding style compliant message could be "core: Express -1 as ~0 in thread_status_t cast" (The first word indicates the core area).

By the way, that means I'll need a 2nd ACK, because it is touching core.

[chrysn]

Out of curiosity on the drilling down side: What's the exact setup you used, like, which version of LLVM, and which version of c2rust? I've tested various more recent versions of c2rust (its 0.18 and its HEAD) with LLVM-15, and neither of them showed me this as an issue while building the riot-wrappers tests.

[SimonIT]

My c2rust version is: C2Rust unknown (0ba4903f3 2023-09-18) dirty 1 modification
My llvm version is: 14.0.0

Should I make a new commit with a new message? (I was normally using conventional commit style for them, hence the fix)

It has to do something with the device because for other boards, it builds perfectly fine

[chrysn]

It has to do something with the device because for other boards, it builds perfectly fine

That'd be extremely weird ... can you try the same test on your machine with BOARD=nrf52840dk? (If that works, try make distclean or remove your bin directory).

Should I make a new commit with a new message?

Yes, please git commit --amend and force push the new commit.

I was normally using conventional commit style for them, hence the fix

Yeah ... it's one of those things that seem to catch on just because someone picked a very presumptuous name for it :-/

[SimonIT]

Okay, interestingly the error appears on both boards only when building with our own branches https://github.com/ATACAMA-Project/rust-riot-sys/tree/NOG and https://github.com/ATACAMA-Project/rust-riot-wrappers/tree/7-nanocoap_sock-wrapper

[maribu]

Thx, the change IMO even makes the code more readable.

[riot-ci]

Murdock results

✔️ PASSED

d778e2e core: Express -1 as ~0 in thread_status_t cast

Success	Failures	Total	Runtime
7937	0	7937	15m:56s

Artifacts

• Documentation preview

[chrysn]

bors merge

[bors]

Build failed:

• static-tests

[maribu]

#19978 should fix the static test failure. It is a pity that the version of codespell run by bors is not pinned to what we use in our CI static test target. That way this kind of breakage wouldn't pop up unexpectedly, but only when we update the CI tools.

[benpicco]

bors merge

[SimonIT]

For documentation purposes:
We removed --translate-const-macros in rust-riot-sys to avoid another problem. So instead of this:

pub const STATUS_NOT_FOUND: libc::c_int = -(1 as libc::c_int);

pub const unsafe fn macro_STATUS_NOT_FOUND() -> thread_status_t {
    let mut result: thread_status_t = STATUS_NOT_FOUND as thread_status_t;
    return result;
}

which successful compiles, this:

pub const unsafe fn macro_STATUS_NOT_FOUND() -> thread_status_t {
    let mut result: thread_status_t = -1 as thread_status_t;
    return result;
}

was generated

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• Murdock
• static-tests
• tools-build-success