[libcxx-commits] [PATCH] D152624: [libc++][format] Improves diagnostics.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

Could you split this patch into a patch that changes the error messages (e.g. reformulation, capitalization, etc), another one that adds additional parsing logic to improve the error messages and one that changes the bracket handling? I feel like there's more than one orthogonal things in this patch, especially since part of it requires an ABI change that goes mostly unnoticed otherwise.



================
Comment at: libcxx/include/__format/parser_std_format_spec.h:382-383
+        return __begin;
+    } else if (std::is_constant_evaluated() && __parse_alternate_form(__begin))
+      std::__throw_format_error("The format specifier does not allow the alternate form option");
 
----------------
Can you use braces consistently after the `else`? Otherwise it becomes really hard to distinguish what is in the `else` given the nesting.


================
Comment at: libcxx/include/__format/parser_std_format_spec.h:442-443
+  //
+  // For some types all valid options need a second validation run, like
+  // a boolean types.
+  //
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152624