-Zc-char-type=unsigned|signed|default flag for c_char->u8/i8 selection override #138290
Conversation
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
rustbot
added
S-waiting-on-review
| @@ -470,5 +473,7 @@ impl CheckCfg { | |||
|
|
|||
| ins!(sym::unix, no_values); | |||
| ins!(sym::windows, no_values); | |||
|
|
|||
| ins!(sym::unsigned_char, empty_values).extend(UnsignedCharVar::all()); | |||
There was a problem hiding this comment.
the cfg needs to be unstable for now
There was a problem hiding this comment.
You mean, to hide it behind unstable feature, right?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
c_char_type unstable feature added and it is now a feature gated cfg.
This comment has been minimized.
This comment has been minimized.
azhogin
force-pushed
the
azhogin/unsigned_char
branch
from
7e5deec to
557e76f
Compare
|
I don't like the name |
For me, unsigned-char looks better and is better greppable and corresponds to Do you think, something like |
|
Yeah, something like |
azhogin
force-pushed
the
azhogin/unsigned_char
branch
2 times, most recently
from
53e9d43 to
6a098a1
Compare
azhogin
changed the title
azhogin
force-pushed
the
azhogin/unsigned_char
branch
from
6a098a1 to
876501b
Compare
| @@ -25,6 +25,8 @@ optimize_for_size = [] | |||
| 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 | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Good point, maybe a feature flag for core would be sufficient for RFL's needs?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 🙂
There was a problem hiding this comment.
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
Changed to |
|
☔ The latest upstream changes (presumably #138996) made this pull request unmergeable. Please resolve the merge conflicts. |
fee1-dead
added
S-experimental
|
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 :) |
#138446
Rust-for-linux request: Rust-for-Linux/linux#355
-Zunsigned-char=unsigned|signed|defaultflag is a target modifier. The flag generates configuration optionunsigned_charto 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.