-Zc-char-type=unsigned|signed|default flag for c_char->u8/i8 selection override by azhogin · Pull Request #138290

-Zc-char-type=unsigned|signed|default flag for c_char->u8/i8 selection override by azhogin · Pull Request #138290 · rust-lang/rust

Conversation

#138446

Rust-for-linux request: Rust-for-Linux/linux#355

-Zunsigned-char=unsigned|signed|default flag is a target modifier. The flag generates configuration option unsigned_char to be used in primitives.rs c_char-to-i8/u8 selection.

cfg!(unsigned_char = "unsigned")
cfg!(unsigned_char = "signed")
cfg!(unsigned_char = "default")

Also, it needs to correct primitives.rs in libc crate in the same way to have consistent libc::c_char.

r? @fee1-dead

rustbot has assigned @fee1-dead.
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

rustbot added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

T-compiler

Relevant to the compiler team, which will review and decide on the PR/issue.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.

labels

Mar 9, 2025

		ins!(sym::unix, no_values);
		ins!(sym::windows, no_values);

		ins!(sym::unsigned_char, empty_values).extend(UnsignedCharVar::all());

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the cfg needs to be unstable for now

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean, to hide it behind unstable feature, right?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, using cfg(unsigned_char) should require using an unstable feature. there are other cfgs like this (I do not remember which) where you can look at how to do it

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c_char_type unstable feature added and it is now a feature gated cfg.

This comment has been minimized.

I don't like the name unsigned-char because the flag configures c_char and not c_uchar. What is the reason behind the name?

I don't like the name unsigned-char because the flag configures c_char and not c_uchar. What is the reason behind the name?

For me, unsigned-char looks better and is better greppable and corresponds to -funsigned-char in gcc/clang, but yes, has some ambigity.

Do you think, something like -Zc-char=unsigned|signed would be better?

Yeah, something like c-char or c-char-type would be better IMO

azhogin changed the title ~~-Zunsigned-char=unsigned|signed|default flag for c_char->u8/i8 selection override~~ -Zc-char-type=unsigned|signed|default flag for c_char->u8/i8 selection override

Mar 13, 2025

		debug_refcell = []
		# Make `TypeId` store a reference to the name of the type, so that it can print that name.
		debug_typeid = []
		#Allow -Zc-char-type='unsigned\|signed\|default' for c_char override

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need a compiler flag? As far as I can see, this (and the other change in library/core/src/ffi/primitives.rs) are the only things actually needed. Given libcore is precompiled, I don't understand what effect -Zc-char-type could actually have.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, maybe a feature flag for core would be sufficient for RFL's needs?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option also need to be consistent with libc c_char. With flag, this consistency will be provided by target modifiers system.
May feature flag mechanics secure consistency of core c_char with libc c_char?
Is there an example of existing feature flag with similar logic?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Rust compiler doesn't have special knowledge of the libc crate so it's not really in a position to enforce that. Maybe libc should just re-export the type from core::ffi?

cc @tgross35 since you've been working on the libc crate lately 🙂

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to reexport core::ffi for now and then just drop the libc types entirely for 1.0, but we can't make these changes yet due to MSRV and compat concerns. That should eventually be able to happen though so I wouldn't make any decisions here based on libc, IOW a library flag seems reasonable 👍 (assuming the compiler doesn't need knowledge of this for whatever reason).

Some more details: rust-lang/libc#4257

Yeah, something like c-char or c-char-type would be better IMO

Changed to -Zc-char-type flag and c_char_type cfg & feature.

fee1-dead added S-experimental

Status: Ongoing experiment that does not require reviewing and won't be merged in its current state.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Mar 30, 2025

Marking as S-experimental as this is a draft. When you need review please unmark it as draft and change it to S-waiting-on-review :)

I wonder if we can/should expand this? There are also other types we may wish to configure, for example, we would like c_long = isize and c_ulong = usize, but currently those types are u32/u64 depending on whether the platform is 32-bit or 64-bit.

ojeda mentioned this pull request

Jun 4, 2025

67 tasks

Comment on lines +9 to +18

		#![no_core]
		#![crate_type = "rlib"]
		#![feature(intrinsics, rustc_attrs, no_core, lang_items, staged_api)]
		#![stable(feature = "test", since = "1.0.0")]

		#[lang="sized"]
		trait Sized {}
		#[lang = "copy"]
		trait Copy {}
		impl Copy for bool {}

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Labels