[libcxx-commits] [PATCH] D115991: [libc++][format] Adds formatter handle.

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added a comment.

LGTM



================
Comment at: libcxx/include/__format/format_arg.h:49-51
 #ifndef _LIBCPP_HAS_NO_INT128
   __i128,
 #endif
----------------
Mordante wrote:
> Mordante wrote:
> > vitaut wrote:
> > > Not directly related to this diff but this seems to make the ABI depend on `_LIBCPP_HAS_NO_INT128`. A better approach would be to always define these enumerators but only use them conditionally.
> > Thanks for the suggestion! I'll do that in a separate patch.
> Applied your suggestion in D116120.
Great, thank you!


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_tests.h:122
+      }
+      end = begin + strlen(begin);
+      break;
----------------
Mordante wrote:
> vitaut wrote:
> > Looks like `begin` may be unterminated in some cases because `buffer` is uninitialized and `to_chars` doesn't nul-terminate.
> This should be save since this code is only executed when `type == 2` and begin is always initialized with a string literal.
Missed that, sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115991