Revert vita's c_char back to i8 #136667

pheki · 2025-02-07T03:00:52Z

Description

Hi!

#132975 changed the definition of c_char from i8 to u8 for most ARM targets. While that would usually be correct, VITASDK uses signed chars by default. The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / VITADSK, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's gcc and we set TARGET_CC and TARGET_CXX in cargo vita for build scripts using cc.

I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the libc side and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

rustbot · 2025-02-07T03:01:00Z

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

workingjubilee · 2025-02-07T04:23:34Z

is there a bug open against clang?

workingjubilee · 2025-02-07T04:24:14Z

( or should someone else be fixing the clang-side issue? )

pheki · 2025-02-08T04:15:28Z

is there a bug open against clang?

I don't think so. In my understanding it doesn't have support for the Vita specifically, nor have I seen evidence of people using Clang for the Vita. I investigated a bit on what would it take to fix it there, AFAIK this would be the best way:

Adding support for the Vita as a separate OSType. I have no idea if they have a policy for new targets.
Make isSignedCharDefault return true for the Vita.

I'm not sure if the changes would need to be tested. To be able to test these changes, someone would need to figure out:

Linker scripts to generate the ELF correctly
Other compiler/linker options (e.g. for custom sysroot)
The usual process for going from an ELF into a VPK (installable package).

Overall it doesn't seem that complicated and it would be great if someone figured it out, but at this moment I'm personally more interested in fixing the Rust toolchain (which doesn't use Clang, only LLVM). To be honest, I feel a bit awkward creating a bug report for a platform that's not yet supported 😅

ibraheemdev · 2025-02-11T02:24:19Z

I'm not the best person to review this. r? libs

cuviper · 2025-02-11T18:55:14Z

@pheki, I see that you are one of the target maintainers, so let's check with the others:

@nikarh and @zetanumbers, any comment?

nikarh · 2025-02-11T19:26:42Z

@pheki, I see that you are one of the target maintainers, so let's check with the others:

@nikarh and @zetanumbers, any comment?

Agree with @pheki.
VitaSDK uses signed char by default, and vita target requires VitaSDK toolchain.
Considering that, the easiest solution for us would be changing char to signed in rust-lang/rust and rust-lang/libc for this target.

zetanumbers · 2025-02-12T10:34:56Z

pheki, I see that you are one of the target maintainers, so let's check with the others:

nikarh and zetanumbers, any comment?

LGTM. I remember signed by default char being intentional. VITASDK solely uses (old patched version of) gcc, and I am not aware of any existing llvm toolchain for vita.

cuviper · 2025-02-13T20:04:38Z

Thanks everyone! I'm marking this blocked until the libc update is ready to go with it.

@rustbot blocked

pheki · 2025-02-28T01:45:27Z

libc PR is merged and a new version (0.2.170) has been released.

About the comments, I tried to keep them grouped with the platforms they're referring to

@rustbot ready

cuviper · 2025-03-06T22:14:27Z

I see that libc was already updated in #137463 too.

@bors r+ rollup

bors · 2025-03-06T22:14:29Z

📌 Commit 93ef808 has been approved by cuviper

It is now in the queue for this repository.

…viper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

…kingjubilee Rollup of 12 pull requests Successful merges: - rust-lang#136667 (Revert vita's c_char back to i8) - rust-lang#136780 (std: move stdio to `sys`) - rust-lang#137107 (Override default `Write` methods for cursor-like types) - rust-lang#137363 (compiler: factor Windows x86-32 ABI impl into its own file) - rust-lang#137528 (Windows: Fix error in `fs::rename` on Windows 1607) - rust-lang#137537 (Prevent `rmake.rs` from using unstable features, and fix 3 run-make tests that currently do) - rust-lang#137777 (Specialize `OsString::push` and `OsString as From` for UTF-8) - rust-lang#137832 (Fix crash in BufReader::peek()) - rust-lang#137904 (Improve the generic MIR in the default `PartialOrd::le` and friends) - rust-lang#138115 (Suggest typo fix for static lifetime) - rust-lang#138125 (Simplify `printf` and shell format suggestions) - rust-lang#138129 (Stabilize const_char_classify, const_sockaddr_setters) r? `@ghost` `@rustbot` modify labels: rollup

…viper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136667 (Revert vita's c_char back to i8) - rust-lang#137107 (Override default `Write` methods for cursor-like types) - rust-lang#137777 (Specialize `OsString::push` and `OsString as From` for UTF-8) - rust-lang#137832 (Fix crash in BufReader::peek()) - rust-lang#137904 (Improve the generic MIR in the default `PartialOrd::le` and friends) - rust-lang#138115 (Suggest typo fix for static lifetime) - rust-lang#138125 (Simplify `printf` and shell format suggestions) - rust-lang#138129 (Stabilize const_char_classify, const_sockaddr_setters) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136667 - vita-rust:revert-vita-c-char, r=cuviper Revert vita's c_char back to i8 # Description Hi! rust-lang#132975 changed the definition of `c_char` from i8 to u8 for most ARM targets. While that would usually be correct, [VITASDK uses signed chars by default](https://github.com/vitasdk/buildscripts/blob/master/patches/gcc/0001-gcc-10.patch#L33-L34). The Clang definitions are incorrect because Clang is not (yet?) supported by the vita commmunity / `VITADSK`, On the Rust side, the pre-compiled libraries the user can link to are all compiled using vita's `gcc` and [we set `TARGET_CC` and `TARGET_CXX`](https://github.com/vita-rust/cargo-vita/blob/d564a132cbd43947118c0d6d0ebfbea7d1dd7fa7/src/commands/build.rs#L230) in `cargo vita` for build scripts using `cc`. I'm creating it as a draft PR so that we can discuss it and possibly get it approved here, but wait to merge the [libc side](rust-lang/libc#4258) and get a libc version first, as having the definitions out of sync breaks std. As a nightly-only target it can be confusing/frustrating for new users when the latest nightly, which is the default, is broken.

Revert vita's c_char back to i8

93ef808

rustbot assigned ibraheemdev Feb 7, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 7, 2025

rustbot assigned cuviper and unassigned ibraheemdev Feb 11, 2025

cuviper mentioned this pull request Feb 12, 2025

fix: Revert vita's c_char back to i8 rust-lang/libc#4258

Merged

3 tasks

rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 13, 2025

kleisauke mentioned this pull request Feb 19, 2025

Bump Rust toolchain to nightly-2025-02-18 and undo Rust ABI test xfail pyodide/pyodide#5249

Draft

1 task

pheki marked this pull request as ready for review February 28, 2025 01:44

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Feb 28, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 6, 2025

workingjubilee mentioned this pull request Mar 7, 2025

Rollup of 12 pull requests #138146

Closed

matthiaskrgr mentioned this pull request Mar 7, 2025

Rollup of 8 pull requests #138151

Merged

jhpratt mentioned this pull request Mar 7, 2025

Rollup of 13 pull requests #138152

Closed

bors merged commit 0b151c6 into rust-lang:master Mar 7, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 7, 2025

taiki-e mentioned this pull request Mar 8, 2025

c_char signedness doesn't match with Clang's default on various no-std and tier 3 targets #129945

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revert vita's c_char back to i8 #136667

Revert vita's c_char back to i8 #136667

pheki commented Feb 7, 2025 •

edited

Loading

rustbot commented Feb 7, 2025

workingjubilee commented Feb 7, 2025

workingjubilee commented Feb 7, 2025

pheki commented Feb 8, 2025

ibraheemdev commented Feb 11, 2025

cuviper commented Feb 11, 2025

nikarh commented Feb 11, 2025 •

edited

Loading

zetanumbers commented Feb 12, 2025 •

edited

Loading

cuviper commented Feb 13, 2025

pheki commented Feb 28, 2025

cuviper commented Mar 6, 2025

bors commented Mar 6, 2025

Revert vita's c_char back to i8 #136667

Revert vita's c_char back to i8 #136667

Conversation

pheki commented Feb 7, 2025 • edited Loading

Description

rustbot commented Feb 7, 2025

workingjubilee commented Feb 7, 2025

workingjubilee commented Feb 7, 2025

pheki commented Feb 8, 2025

ibraheemdev commented Feb 11, 2025

cuviper commented Feb 11, 2025

nikarh commented Feb 11, 2025 • edited Loading

zetanumbers commented Feb 12, 2025 • edited Loading

cuviper commented Feb 13, 2025

pheki commented Feb 28, 2025

cuviper commented Mar 6, 2025

bors commented Mar 6, 2025

pheki commented Feb 7, 2025 •

edited

Loading

nikarh commented Feb 11, 2025 •

edited

Loading

zetanumbers commented Feb 12, 2025 •

edited

Loading