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

[Mark de Wever via Phabricator via libcxx-commits]

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

Thanks for the review.



================
Comment at: libcxx/include/__format/format_arg.h:49-51
 #ifndef _LIBCPP_HAS_NO_INT128
   __i128,
 #endif
----------------
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.


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_tests.h:122
+      }
+      end = begin + strlen(begin);
+      break;
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115991