-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[libc][wctype] Add cmake flag for configuring wctype implementation used #170531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
@llvm/pr-subscribers-libc Author: Marcell Leleszi (mleleszi) Changes#170525 Based on our RFC, we are starting the implementation of the wctype header with C.UTF8 support. This implementation will increase the binary size by approximately ~70KB, so this PR introduces a flag so that this is configurable, allowing the user to choose between a simple ASCII implementation or the full Unicode one. Full diff: https://github.com/llvm/llvm-project/pull/170531.diff 1 Files Affected:
diff --git a/libc/config/config.json b/libc/config/config.json
index cfbe9a43948ea..a7844e4fe2dd1 100644
--- a/libc/config/config.json
+++ b/libc/config/config.json
@@ -130,5 +130,11 @@
"value": true,
"doc": "Add nullptr checks in the library's implementations to some functions for which passing nullptr is undefined behavior."
}
+ },
+ "wctype": {
+ "LIBC_CONF_WCTYPE_MODE": {
+ "value": "LIBC_WCTYPE_MODE_ASCII",
+ "doc": "The implementation used for wctype, acceptable values are LIBC_WCTYPE_MODE_ASCII and LIBC_WCTYPE_MODE_UTF8."
+ }
}
}
|
bassiounix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add some description to let people know how to utilize this option with its possible values.
Please add some description in the docs like:
llvm-project/libc/docs/configure.rst
Line 22 in 035f811
$> cmake <other build options> -DLIBC_CONF_PRINTF_DISABLE_FLOAT=ON <more options> llvm-project/libc/docs/configure.rst
Line 40 in 035f811
- ``LIBC_CONF_PRINTF_DISABLE_FLOAT``: Disable printing floating point values in printf and friends.
|
The config flag looks good, if you run the cmake it should automatically update the documentation. |
I had no idea it's automatically generated. I thought we write the docs ourselves. |
|
The configuration documentation and the lists of implemented functions are both automatically generated: https://github.com/llvm/llvm-project/blob/main/libc/utils/docgen/docgen.py |
|
Done, regenerated docs |
bassiounix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…sed (llvm#170531) llvm#170525 Based on our [RFC](https://discourse.llvm.org/t/rfc-libc-wctype-header-implementation/88941/10), we are starting the implementation of the wctype header with C.UTF8 support. This implementation will increase the binary size by approximately ~70KB, so this PR introduces a flag so that this is configurable, allowing the user to choose between a simple ASCII implementation or the full Unicode one.
#170525
Based on our RFC, we are starting the implementation of the wctype header with C.UTF8 support. This implementation will increase the binary size by approximately ~70KB, so this PR introduces a flag so that this is configurable, allowing the user to choose between a simple ASCII implementation or the full Unicode one.