Fix feature dependency

[gmmyung]

• When adding functions like strtol, strtoll, strtoul, strtoull, and isspace, the functions can share a lot of helper functions. This makes feature gating each function hard since the dependency of each feature is intertwined with each other .
  I added changes to leave the internal implementation of a func as r_func that is pub(crate) regardless of the feature gate, and then feature gated the actual func.

• Also, type definitions are moved to a separate file: ctype.rs

• Added implementations of strtoll, stroull, strtoimax, strtoumax, isspace, isdigit, isalpha, isupper

[eldruin]

Interesting. What do you thing @thejpster ?

[gmmyung]

rust-lang/rust#100184

Wut??? Am I missing something here?
I am not sure why the tests are flaking

[eldruin]

Could you update the clippy version to 1.75.0 here in the .github/workflows/build.yml?

tinyrlibc/.github/workflows/build.yml

Line 106 in e9a3798

toolchain: 1.64.0 # clippy is too much of a moving target at the moment

[jonathanpallant]

• I see this error when building locally (on an Aarch64 Apple Mac):

warning: [email protected]: ./src/snprintf.c:68:26: warning: incompatible redeclaration of library function 'strtoul' [-Wincompatible-library-redeclaration]
warning: [email protected]: extern unsigned long int strtoul(const char* str, const char* restrict* endptr, int base);
warning: [email protected]:                          ^
warning: [email protected]: ./src/snprintf.c:68:26: note: 'strtoul' is a builtin with type 'unsigned long (const char *, char **, int)'
warning: [email protected]: 1 warning generated.

• The Exxx constants should be in the errno module otherwise they clog up the front page of the docs.

• The r_xxx functions appear in the public API (i.e. in the cargo doc output) and that's probably not right as they are an internal implementation detail?

Instead of adding r_xxx functions, you could cfg-gate an attribute like export_as. This means the function always exists (and it's fine if it's extern "C" but only Rust code calls it), but if you turn on the flag then it also appears as that specific named symbol. That probably cuts down on code duplication but also allows the user to decide which symbols they want and which symbols they do not want.

[gmmyung]

Where did these numbers come from? And how can we be sure they match the errno.h header file on any given platform?

I guess I need to get rid of the errno placeholder. Sorry for the implementation being all over the place; This was for my own project, and I thought it would be great if it merges upstream.

The r_xxx functions appear in the public API (i.e. in the cargo doc output) and that's probably not right as they are an internal implementation detail?

This was why r_xxx funcs were set as pub(crate).I am not able to check if the functions actually show up on docs because of my schedule now, but it should be hidden to the end user(Well I just found out my mistake) And the symbol names won’t matter since r_xxx functions have mangled names.

you could cfg-gate an attribute like export_as.

I am not aware of that feature, could you please leave a link or an example of its use? Thanks for your feedback!

[gmmyung]

warning: [email protected]: ./src/snprintf.c:68:26: warning: incompatible redeclaration of library function 'strtoul' [-Wincompatible-library-redeclaration]
warning: [email protected]: extern unsigned long int strtoul(const char* str, const char* restrict* endptr, int base);
warning: [email protected]: ^
warning: [email protected]: ./src/snprintf.c:68:26: note: 'strtoul' is a builtin with type 'unsigned long (const char *, char **, int)'
warning: [email protected]: 1 warning generated.

This build warning is also shown at the main branch too.

[gmmyung]

Things are all cleaned up. What do you think @jonathanpallant ?

[gmmyung]

Everything should be okay, except the fact that disabling feature itoa breaks snprintf.
A possible solution: mangle the function name ourselves, and inject the symbols into sprintf.c using build script macro definitions?

[thejpster]

Looks good at first glance - I'll give it a second pass when I can.

Does defining X as X in the build script actually do anything?

[gmmyung]

The itoa function symbol is set to itoa when the itoa feature is enabled, or set to tinyrlibc_itoa when the feature is disabled. This info is passed to snprintf.c using the build script.

 #[cfg_attr(not(feature = "itoa"), export_name = "tinyrlibc_itoa")]
 #[cfg_attr(feature = "itoa", no_mangle)]
 pub unsafe extern "C" fn itoa(i: i64, s: *mut CChar, s_len: usize, radix: u8) -> i32 {

The same is applied to utoa and strtoul.

[thejpster]

Yes but is it necessary to define a macro X (e.g. "iota") which has the value X (e.g. "iota"). You could just not define it and you'd get the same result, right? With less code? Maybe I'm missing something.

[thejpster]

I'm not sure the .clone() calls are required either.

[gmmyung]

Yes but is it necessary to define a macro X (e.g. "iota") which has the value X (e.g. "iota"). You could just not define it and you'd get the same result, right? With less code? Maybe I'm missing something.

I'm not sure the .clone() calls are required either.

Yeah, you're right! Changes are pushed.

[thejpster]

I think we're good to go. Just a quick question as I'm on my phone - did the errno stuff stay in or get removed? Because there's a flag for it, but also there's todos saying they should set errno. Sorry, I couldn't find a search button.

[gmmyung]

Oops, fixed. I blindly thought rust had some kind of unused feature warning.

[thejpster]

I'll merge this tomorrow. Note to self - squash it.

Thank you for your PR! It is much appreciated.