Skip to content

[libc++][z/OS] Disable portion of formatter.char.funsigned-char.pass.cpp for no unicode #94044

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

Merged
merged 2 commits into from
Jun 12, 2024

Conversation

zibi2
Copy link
Contributor

@zibi2 zibi2 commented May 31, 2024

This PR carves out small portion of the test in subject to avoid the following failure when unicode is not available.

# | Assertion failure: result == expected .../formatter.char.funsigned-char.pass.cpp 56
# |
# | Format string   ?}
# | Expected output '\x{80}'
# | Actual output   '�'

This was traced down to different definition of __code_point_view::__consume() under macro_LIBCXX_HAS_NO_UNICODE which is called inside __formatter::__escape(). The __consume() returns __ok and code assumes that escaped sequence was already written but it is not., thus the failure. Here is the snippen code we fall into:

    typename __unicode::__consume_result __result = __view.__consume();
    if (__result.__status == __unicode::__consume_result::__ok) {
      __escape = __formatter::__is_escaped_sequence_written(__str, __result.__code_point, __escape, __mark);

@zibi2 zibi2 requested a review from mordante May 31, 2024 20:18
@zibi2 zibi2 self-assigned this May 31, 2024
@zibi2 zibi2 requested a review from a team as a code owner May 31, 2024 20:18
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label May 31, 2024
@llvmbot
Copy link
Member

llvmbot commented May 31, 2024

@llvm/pr-subscribers-libcxx

Author: Zibi Sarbinowski (zibi2)

Changes

This PR carves out small portion of the test in subject to avoid the following failure when unicode is not available.

# | Assertion failure: result == expected .../formatter.char.funsigned-char.pass.cpp 56
# |
# | Format string   ?}
# | Expected output '\x{80}'
# | Actual output   '�'

This was traced down to different definition of __code_point_view::__consume() under macro_LIBCXX_HAS_NO_UNICODE which is called inside __formatter::__escape(). The __consume() returns __ok and code assumes that escaped sequence was already written but it is not., thus the failure. Here is the snippen code we fall into:

    typename __unicode::__consume_result __result = __view.__consume();
    if (__result.__status == __unicode::__consume_result::__ok) {
      __escape = __formatter::__is_escaped_sequence_written(__str, __result.__code_point, __escape, __mark);

Full diff: https://github.com/llvm/llvm-project/pull/94044.diff

1 Files Affected:

  • (modified) libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char.funsigned-char.pass.cpp (+4-2)
diff --git a/libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char.funsigned-char.pass.cpp b/libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char.funsigned-char.pass.cpp
index 9c31ecad85eac..643efe00e3fc2 100644
--- a/libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char.funsigned-char.pass.cpp
+++ b/libcxx/test/std/utilities/format/format.formatter/format.formatter.spec/formatter.char.funsigned-char.pass.cpp
@@ -71,16 +71,18 @@ void test() {
 #if TEST_STD_VER > 20
   test(STR(R"('\u{0}')"), STR("?}"), '\x00');
   test(STR("'a'"), STR("?}"), 'a');
+#  ifndef TEST_HAS_NO_UNICODE
   if constexpr (std::same_as<CharT, char>) {
     test(STR(R"('\x{80}')"), STR("?}"), '\x80');
     test(STR(R"('\x{ff}')"), STR("?}"), '\xff');
   }
-#  ifndef TEST_HAS_NO_WIDE_CHARACTERS
+#    ifndef TEST_HAS_NO_WIDE_CHARACTERS
   else {
     test(STR(R"('\u{80}')"), STR("?}"), '\x80');
     test(STR("'\u00ff'"), STR("?}"), '\xff');
   }
-#  endif // TEST_HAS_NO_WIDE_CHARACTERS
+#    endif // TEST_HAS_NO_WIDE_CHARACTERS
+#  endif // TEST_HAS_NO_UNICODE
 #endif   // TEST_STD_VER > 20
 
   test(STR("10000000"), STR("b}"), char(128));

@zibi2
Copy link
Contributor Author

zibi2 commented May 31, 2024

The other alternative would be to diable entire test for no unicode mode only which seems like a big hammer for me.

@zibi2 zibi2 changed the title [libc++][z/OS] Disable porsion of formatter.char.funsigned-char.pass.cpp for no unicode [libc++][z/OS] Disable portion of formatter.char.funsigned-char.pass.cpp for no unicode May 31, 2024

This comment was marked as resolved.

@zibi2 zibi2 requested review from redstar and fanbo-meng May 31, 2024 20:22
@zibi2
Copy link
Contributor Author

zibi2 commented Jun 6, 2024

ping @mordante

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM!

@zibi2 zibi2 merged commit 47afa10 into llvm:main Jun 12, 2024
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants