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

Zibi Sarbino via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Tue Jan 17 09:42:35 PST 2023


zibi marked 2 inline comments as done.
zibi added inline comments.


================
Comment at: libcxx/test/libcxx/utilities/utility/__murmur2_or_cityhash.pass.cpp:27
+    {/* "CityHash" */ "\x43\x69\x74\x79\x48\x61\x73\x68"},
+    {/* "CitYHash" */ "\x43\x69\x74\x59\x48\x61\x73\x68"},
+};
----------------
tahonermann wrote:
> zibi wrote:
> > philnik wrote:
> > > I thought more of just making this `{43, 69, 74, 48, 61, 73, 68}`. Or is there a reason not to do that?
> > Yes, we can do that. I used hex values, otherwise I would need to convert them to decimal values.
> I think the numeric hex escapes in a string literal were a better solution. When `char` is a signed 8-bit type, use of the `initializer_list` will reject narrowing conversions that occur when a code point value above 0x7F is needed.
Yes, you are right. At the moment we don't have anything > 0x7F so we are fine however, future expansion might introduce this issue.

I propose to reverse last commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141623



More information about the libcxx-commits mailing list