[libcxx-commits] [PATCH] D99091: [locale][num_get] Improve Stage 2 of string to float conversion
Tomas Matheson via Phabricator via libcxx-commits
libcxx-commits at lists.llvm.org
Wed Apr 7 12:42:31 PDT 2021
tmatheson marked 2 inline comments as done.
tmatheson added inline comments.
================
Comment at: libcxx/include/locale:524
+
+ if (__ct == __decimal_point) {
+ if (!__in_units)
----------------
Quuxplusone wrote:
> tmatheson wrote:
> > miyuki wrote:
> > > I think it would be better to keep 4-space indentation for consistency with the rest of the file. See https://github.com/llvm/llvm-project/commit/7004d6664efde9d1148ed677649593f989cc6056
> > I agree, but the CI was failing if I didn't clang-format the patch.
> The clang-format CI step is non-fatal; it's just there to tell you what parts fail clang-format. It is not intended as a black-and-white gatekeeper, and requests-for-consistency from real humans should always take precedence over the tool's suggestions — [Asimov's Second Law](https://en.wikipedia.org/wiki/Three_Laws_of_Robotics) applies. :)
>
Sorry my mistake, I thought it had caused one of the failures I had earlier
================
Comment at: libcxx/test/std/localization/locale.categories/category.numeric/locale.num.get/facet.num.get.members/get_float.pass.cpp:55
- {
- const char str[] = "-123";
- std::ios_base::iostate err = ios.goodbit;
----------------
miyuki wrote:
> I think there is a reason behind having many similar-looking blocks: each assert() call is located on a separate line, so an assertion failure message will indicate which assertion failed. Factoring out all checks into a single function will make error messages much less informative.
This was addressed by using a macro for new tests and not refactoring the old tests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D99091/new/
https://reviews.llvm.org/D99091
More information about the libcxx-commits
mailing list