[libcxx-commits] [PATCH] D120448: [libc++][AIX][test] Enable put_double/long_double locale tests

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp:8939
+    std::string INF = "INF";
+    std::string inf_padding25 = "*********************";
+
----------------
I think the general direction in earlier reviews (I think it might have been in D120022), is that for these strings that don't differ between environments, keep the string literal `"INF"` and the padding, instead of folding them into variables.

So here, the only thing that really differs is `inf` vs `INF`, then we would only need that single variable, but keeping the literal strings for everything else.


================
Comment at: libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp:9177
                                     std::string ex(str, iter.base());
-                                    assert(ex == "-inf");
+                                    assert(ex ==  ninf_sign + inf );
                                     assert(ios.width() == 0);
----------------
I guess this is quite nitpicky, but many of these edits seem to be adding more inconsistent whitespace in the test - here there's suddenly two spaces between `==` and `ninf_sign`, and there's a trailing space before the ending parenthesis. I'd prefer if we'd avoid introducing extra inconsistencies.


================
Comment at: libcxx/test/std/localization/locale.categories/category.numeric/locale.nm.put/facet.num.put.members/put_long_double.pass.cpp:10736
+    std::string nan= "nan";
+    std::string NaN = std::string("NAN");
+    std::string nan_padding25 = "**********************";
----------------
Why the extra `std::string()` here? A plain string literal like `"NAN"` should work just fine?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120448