[libcxx-commits] [PATCH] D103413: [libc++][format] Implement Unicode support.

[Mark de Wever via Phabricator via libcxx-commits]

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


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:744-767
+  return 1 + (__c >= 0x1100 &&
+              (__c <= 0x115f ||
+               (__c >= 0x2100 &&
+                (__c <= 0x232a ||
+                 (__c >= 0x2e80 &&
+                  (__c <= 0x303e ||
+                   (__c >= 0x3040 &&
----------------
Mordante wrote:
> vitaut wrote:
> > A sequence of ifs would likely be more readable.
> Agreed. I originally had a sequence of `if`s, but that was a bit slow. I used this to speed things up. However I'll look whether I can use `if`s in a smarter fashion to improve readability.
I tried some different approaches, but they lead to performance degradation. (I also tested the version used in libfmt.)
I think the easiest way to solve this is by improving the formatting,so I went for that route.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:832
+ *
+ * @tparam __truncate When the last parsed code point is a multi-column output
+ *                    it may exceed the @a __threshold by one. The behaviour
----------------
Mordante wrote:
> vitaut wrote:
> > Why not set `__threshold` to the maximum value to disable it instead of introducing another flag?
> That can give the issue, where the result can be off by one. I'll see whether I can find a way to improve this code.
Setting it to maximum is less efficient. For example, when a width of 10 is given it's not needed to parse a 1024 code unit input. However it's possible to increase the wanted width + 1 to get the same effect as the current `__truncate` argument. This still may scan one code unit more than required, but I consider that not an issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103413