[libcxx-commits] [PATCH] D141623: [SystemZ][z/OS] Fix cityhash lit for EBCDIC

Fri Jan 13 12:27:14 PST 2023

zibi marked an inline comment as done.
zibi added inline comments.

================
Comment at: libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp:31
   const std::pair<std::string, uint64_t> TestCases[] = {
-      {"abcdefgh", CHOOSE_BY_ENDIANESS(0x4382a8d0fe8edb17ULL, 0xca84e809bef16fbcULL)},
-      {"abcDefgh", CHOOSE_BY_ENDIANESS(0xecefb080a6854061ULL, 0xd7feb824250272dcULL)},
-      {"CityHash", CHOOSE_BY_ENDIANESS(0x169ea3aebf908d6dULL, 0xea8cef3ca6f6e368ULL)},
-      {"CitYHash", CHOOSE_BY_ENDIANESS(0xe18298a2760f09faULL, 0xf33a7700bb7a94a8ULL)},
+      {"abcdefgh",
+       CHOOSE_BY_ENDIANESS(0x4382a8d0fe8edb17ULL, CHOOSE_BY_CODEPAGE(0xca84e809bef16fbcULL, 0xb6eaf957b2d7afdfULL))},
----------------
philnik wrote:
> Couldn't we just change these to char arrays with numbers instead of a string that might be different between platforms? That should make the Test platform independent.
Thank you for looking at it and your comments.

Is the new patch is what you meant?  I think we want the numbers to be more than just what char can hold.  

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D141623/new/

https://reviews.llvm.org/D141623