-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[libc][z/OS] Remove ASCII trick to fix EBDIC std::from_char #116078
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: Zibi Sarbinowski (zibi2) ChangesThis PR will fix the following lit in all EBCDIC variations on z/OS: The trick to test for Full diff: https://github.com/llvm/llvm-project/pull/116078.diff 1 Files Affected:
diff --git a/libc/src/__support/high_precision_decimal.h b/libc/src/__support/high_precision_decimal.h
index ac11649d1d1686..20088d6d797910 100644
--- a/libc/src/__support/high_precision_decimal.h
+++ b/libc/src/__support/high_precision_decimal.h
@@ -350,7 +350,8 @@ class HighPrecisionDecimal {
if (!saw_dot)
this->decimal_point = total_digits;
- if (num_cur < num_len && ((num_string[num_cur] | 32) == 'e')) {
+ if (num_cur < num_len &&
+ (num_string[num_cur] == 'e' || num_string[num_cur] == 'E')) {
++num_cur;
if (isdigit(num_string[num_cur]) || num_string[num_cur] == '+' ||
num_string[num_cur] == '-') {
|
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
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.
Thanks for catching this, I'll see if there's anywhere else that could use this sort of cleanup.
CI failed twice at unrelated |
@zibi2 what's a target triple we can use to test EBDIC? @michaelrj-google is making more related changes in #110574. |
(Zibi's off for a couple days. I'll answer in the meantime)
This would be a specific build of libc++ for EBCDIC. We haven't tried enabling EBCDIC on other platforms. In theory it should work. |
This PR will fix the following lit in all EBCDIC variations on z/OS:
std/utilities/charconv/charconv.from.chars/floating_point.pass.cpp
The trick to test for
e
andE
is working only in ASCII.The fix is to simply test for both lower and upper case exponent letter
e
andE
respectfully.