In D107755#2936908, @mstorsjo wrote:

In D107755#2936886, @NancyWang2222 wrote:

hi do you know Why this patch was closed?

I applied it already as it was approved, as I thought you said you can fix any remaining z/OS related details in separate patches later?

hi do you know Why this patch was closed?

hi There is the failing test case libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp , this test case is not in https://reviews.llvm.org/D107124 and https://reviews.llvm.org/D105910 , the failure seems not related to this patch. it fails same without this patch. we will investigate separately. rest of tests in https://reviews.llvm.org/D107124 and https://reviews.llvm.org/D105910 passed. Thanks for the fixes.
Assertion failed: Result == E, file: /plex/wangn/llvm/dev/llvm-project/libcxx/test/std/input.output/filesystems/class.path/path.member/path.append.pass.cpp, line: 250
CEE5207E The signal SIGABRT was received.

The changes look ok to me although I havent tried it on z/OS as long as build pass. I will fix in separate patch if there is an issue on z/OS. just want to confirm those assert wont be run on z/OS and assert macros are empty. I will need some time to download patch and test it out.

Let me know if this is ready, I will need to test it on z/OS.

hi Any suggestion how we should fix those test case, I can add unsupport on z/OS.

@Quuxplusone Thanks Arthur, I can remove delete macro. let me know how windows works if need me to check windows as well.

@Quuxplusone hi Arthur, Can you review ?

Hi I only updated 3 test cases , I didnt modify test cases in https://reviews.llvm.org/D105910 , because @DanielMcIntosh-IBM did some investigation already in those test cases, I will let him handle it. another reason is I am unable to test windows if it requires modify macros for windows as well

Thanks. will look into how ASSERT_VIA_OVERRIDDEN_NEW works

ping :)

@ldionne hi Louis, any question regarding to this PR, I noticed you reviewed similar changes before. Thanks.

In D106151#2890682, @ldionne wrote:

LGTM, but please add short comments (1-2 lines) explaining why the tests are unsupported. I don't need to see this again.

In D106153#2891424, @ldionne wrote:

In D106153#2890692, @ldionne wrote:

LGTM, but please add a 1-2 line comment for each UNSUPPORTED explaining why it fails. You can copy/paste the comment, it's fine. Actually, it even helps cause you can then grep for:

// <the-comment>
// UNSUPPORTED: target={{.+}}-zos{{.*}}

and you'll find all tests that are marked as unsupported for the same reason.

@muiez

Please read the comments even when you get an approval. https://reviews.llvm.org/rG7704fedfff6ef5676adb6415f3be0ac927d1a746 didn't apply my requested change. I LGTM'd the patch because I trusted you'd apply the feedback and didn't want to block you until I could look at it again -- not because no changes were required.

hi Arthur O'Dwyer , I have updated test, can you review it? Thanks

change test cases to be unsupported on z/OS

add unsupported on zOS for 2 failing test cases

hi Arthur O'Dwyer, I am thinking if we can add unsupported cxx-03 for both test cases with current resolution. adding u'some test' or u'a' is right syntax for uchar16_t , same for utf8_t type. any thoughts about this ?

yeah. I also have same question as Sean mentioned. codecvt class for UTF16/32_t starts from c++11, utf8_t started from c++20. we shouldn't run with c++03. the error is expected.

do we know why build are failing ?

In D106151#2883266, @Quuxplusone wrote:

LGTM (if icky) for the char8_t-based tests.
For the other two tests that are currently failing buildkite in C++03 mode (because u's' is a syntax error in C++03), personally I would recommend leaving the code alone and just UNSUPPORTED'ing them on zOS. They're already marked with

// This test runs in C++20, but we have deprecated codecvt<char(16|32), char, mbstate_t> in C++20.

which indicates that it's not too important to make this stuff work on "new" platforms — in fact we're actively deprecating/breaking it on "old" platforms and it might be completely moot by the time C++23 ships.

hi Arthur O'Dwyer , can you kindly review it again? Thanks

ping :)

use new assert pattern so that we can keep old tests

1. added assert(std::char_traits<char>::lt('A','a') == !std::char_traits<char>::lt('a','A')); for case insensitive test.
2. make content of 2 test cases same except one testing char type another one testing wchar_t

ping ;) can I get review for this ?

In D98793#2634510, @abhina.sreeskantharajan wrote:

Hi Nancy, can you set this patch as a child revision of https://reviews.llvm.org/D98864 which sets the maximum alignment.