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

[Victor Zverovich via Phabricator via libcxx-commits]

vitaut added inline comments.


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


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_tests.h:71-75
+#ifndef _LIBCPP_NO_EXCEPTIONS
+      throw std::format_error("The format-spec type has a type not supported for a status argument");
+#else
+      std::abort();
+#endif
----------------
Maybe move error reporting to a function to have `_LIBCPP_NO_EXCEPTIONS` check in one place?


================
Comment at: libcxx/test/std/utilities/format/format.functions/format_tests.h:122
+      }
+      end = begin + strlen(begin);
+      break;
----------------
Looks like `begin` may be unterminated in some cases because `buffer` is uninitialized and `to_chars` doesn't nul-terminate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115991