[libcxx-commits] [PATCH] D112958: [libcxx][NFC] tidy up money_get::__do_get's sign parsing

[Mark de Wever via Phabricator via libcxx-commits]

Mordante accepted this revision.
Mordante added a comment.

LGTM!



================
Comment at: libcxx/include/locale:2969
+                ++__b;
+                __neg = false;
+                if (__psn.size() > 1)
----------------
DanielMcIntosh-IBM wrote:
> Mordante wrote:
> > The original algorithm never sets `__neg` to `false`. Looking at the code it seems the call-site always sets this value to `false`.
> True, however, as you mentioned, the call-site always sets the value to false, so this isn't actually a functional change.
> 
> I'm open to other ideas, but explicitly setting `__neg` to false when we detect a positive seems more illustrative to me, and generally safer.
> 
> Additionally, explicitly setting it to false here means that the only case where we _don't_ set `__neg` is when `__psn.size() == 0 && __nsn.size() == 0` - i.e. when the locale has no means of specifying a positive or negative value. This lets us use the initial value of `__neg` as a default value for that case. 
Fair points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112958