[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-30 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen added a comment.

> @EricWF Do you have time and interest to review this again?

I should ask, do you have time and interest to //commit// this?


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-29 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen added a comment.

@EricWF Do you have time and interest to review this again?


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-28 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen added inline comments.



Comment at: 
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:79
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }

vangyzen wrote:
> This assertion now fails for me.  wexpected is 0xa0 (NBSP), which is correct. 
>  However, np.thousands_sep() is -62, which is 0xc2, which is the first byte 
> of a UTF-8-encoded NBSP.  It looks like the library isn't correctly handling 
> multibyte thousands separators.
D27167 adds support for multibyte strings in decimal_point and thousands_sep.  
With that change, this unit test passes.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-25 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen added inline comments.



Comment at: 
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp:42
+assert(std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);

localeconv()->grouping can become invalid after we reset the locale to "C", so 
duplicate the string into a local.



Comment at: 
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:79
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }

This assertion now fails for me.  wexpected is 0xa0 (NBSP), which is correct.  
However, np.thousands_sep() is -62, which is 0xc2, which is the first byte of a 
UTF-8-encoded NBSP.  It looks like the library isn't correctly handling 
multibyte thousands separators.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-25 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen added a comment.

I'm glad you mentioned a multibyte thousands_sep, because it //is// multibyte 
in fr_FR.UTF-8 on FreeBSD 11.  Specifically, it's a no-break space (U+00A0).


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-25 Thread Eric van Gyzen via Phabricator via cfe-commits 
vangyzen updated this revision to Diff 79322.
vangyzen added a comment.

Handle multibyte thousands_sep; do not reference possibly stale locale data


https://reviews.llvm.org/D26979

Files:
  
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -16,10 +16,8 @@
 
 // char_type thousands_sep() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,45 @@
 }
 }
 {
+char expected;
+wchar_t wexpected;
+assert(std::setlocale(LC_ALL, LOCALE_en_US_UTF_8) != NULL);
+const char *ts = std::localeconv()->thousands_sep;
+expected = *ts;
+int nb = mbtowc(, ts, strlen(ts)+1);
+assert(nb >= 0);
+assert(std::setlocale(LC_ALL, "C") != NULL);
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }
 }
 {
+char expected;
+wchar_t wexpected;
+assert(std::setlocale(LC_ALL, LOCALE_fr_FR_UTF_8) != NULL);
+const char *ts = std::localeconv()->thousands_sep;
+expected = *ts;
+int nb = mbtowc(, ts, strlen(ts)+1);
+assert(nb >= 0);
+assert(std::setlocale(LC_ALL, "C") != NULL);
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == wexpected);
 }
 }
 }
Index: projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ projects/libcxx/test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -16,10 +16,8 @@
 
 // string grouping() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,37 @@
 }
 }
 {
+assert(std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
+free(expected);
 }
 {
+assert(std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8) != NULL);
+char *expected = strdup(std::localeconv()->grouping);
+assert(std::setlocale(LC_NUMERIC, "C") != NULL);
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
+free(expected);
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-23 Thread Eric van Gyzen via cfe-commits 
vangyzen added inline comments.



Comment at: 
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:65
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");

vangyzen wrote:
> EricWF wrote:
> > We should probably assert that `thousands_sep` isn't a multibyte character.
> I don't quite follow.  C defines it as `char*`.  Are you concerned about an 
> implementation defining it as `wchar_t*`?
Oh, I understand now.  Sorry.  I've never really done much with locales, so I'm 
not so fluent with them...especially early on a holiday morning.  :)


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-23 Thread Eric van Gyzen via cfe-commits 
vangyzen added a comment.

In https://reviews.llvm.org/D26979#604013, @EricWF wrote:

> Nope. Feel free to commit after adding the requested assertions.


I would gladly commit it, but I don't have commit access.  I'm quite new here.




Comment at: 
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:65
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");

EricWF wrote:
> We should probably assert that `thousands_sep` isn't a multibyte character.
I don't quite follow.  C defines it as `char*`.  Are you concerned about an 
implementation defining it as `wchar_t*`?


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-23 Thread Eric Fiselier via cfe-commits 
EricWF added a comment.

In https://reviews.llvm.org/D26979#603110, @vangyzen wrote:

> @EricWF, would you like to see anything more from me?  I could set up an LLVM 
> development environment on my Mac if you want me to run the unit tests on Mac.


Nope. Feel free to commit after adding the requested assertions.




Comment at: 
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:65
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");

We should probably assert that `thousands_sep` isn't a multibyte character.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen added a comment.

@EricWF, would you like to see anything more from me?  I could set up an LLVM 
development environment on my Mac if you want me to run the unit tests on Mac.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen updated this revision to Diff 78921.
vangyzen added a comment.

Remove XFAIL on linux-gnu

These tests now pass on Fedora 24.


https://reviews.llvm.org/D26979

Files:
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -16,10 +16,8 @@
 
 // char_type thousands_sep() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,43 @@
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 }
Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -16,10 +16,8 @@
 
 // string grouping() const;
 
-// TODO: investigation needed
-// XFAIL: linux-gnu
-
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +38,43 @@
 }
 }
 {
+const char *expected = "\3\3";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 }
 {
+const char *expected = "\x7F";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen updated this revision to Diff 78919.
vangyzen added a comment.

Fix comment

vim + Caps Lock == :(


https://reviews.llvm.org/D26979

Files:
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -20,6 +20,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +41,43 @@
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 }
Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -20,6 +20,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +41,43 @@
 }
 }
 {
+const char *expected = "\3\3";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 }
 {
+const char *expected = "\x7F";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen updated this revision to Diff 78916.
vangyzen added a comment.

Reset the global locale so it's different than the one we pass
to std::locale l(...).  That way we're testing that we don't accidentally
use the global locale.


https://reviews.llvm.org/D26979

Files:
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -20,6 +20,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +41,43 @@
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 }
Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -1,5 +1,4 @@
-//===--===//
-//
+//===--===
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
@@ -20,6 +19,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +40,43 @@
 }
 }
 {
+const char *expected = "\3\3";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 }
 {
+const char *expected = "\x7F";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+(void) std::setlocale(LC_NUMERIC, "C");
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 }
 }

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen added inline comments.



Comment at: 
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:50
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {

EricWF wrote:
> I think we should reset the global locale so it's different than the one we 
> pass to `std::locale l(...)`. That way we're testing that we don't accidental 
> use the global locale.
Good idea.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric Fiselier via cfe-commits 
EricWF accepted this revision.
EricWF added a reviewer: EricWF.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM minus inline comments.

Thanks for working on this.




Comment at: 
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp:50
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {

I think we should reset the global locale so it's different than the one we 
pass to `std::locale l(...)`. That way we're testing that we don't accidental 
use the global locale.


https://reviews.llvm.org/D26979



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26979: Do not hard-code locale data in unit tests: get it from the OS instead

2016-11-22 Thread Eric van Gyzen via cfe-commits 
vangyzen created this revision.
vangyzen added a subscriber: cfe-commits.
Herald added a subscriber: emaste.

Locale data can vary across platforms.  Rather than hard-code the expected
data in the units tests, get it from the OS via the C API.

This fixes these unit tests on FreeBSD.  It might fix them on Linux, too.
I have not tested other platforms, but I will if I get agreement
on the approach.


https://reviews.llvm.org/D26979

Files:
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
  
test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp

Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/thousands_sep.pass.cpp
@@ -20,6 +20,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +41,41 @@
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 {
+char expected = ',';
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = *std::localeconv()->thousands_sep;
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == ',');
+assert(np.thousands_sep() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.thousands_sep() == L',');
+assert(np.thousands_sep() == expected);
 }
 }
 }
Index: test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
===
--- test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
+++ test/std/localization/locale.categories/facet.numpunct/locale.numpunct.byname/grouping.pass.cpp
@@ -20,6 +20,7 @@
 // XFAIL: linux-gnu
 
 #include 
+#include 
 #include 
 
 #include "platform_support.h" // locale name macros
@@ -40,29 +41,41 @@
 }
 }
 {
+const char *expected = "\3\3";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_en_US_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+}
 std::locale l(LOCALE_en_US_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\3\3");
+assert(np.grouping() == expected);
 }
 }
 {
+const char *expected = "\x7F";
+const char *ret = std::setlocale(LC_NUMERIC, LOCALE_fr_FR_UTF_8);
+if (ret != NULL)
+{
+expected = std::localeconv()->grouping;
+}
 std::locale l(LOCALE_fr_FR_UTF_8);
 {
 typedef char C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 {
 typedef wchar_t C;
 const std::numpunct& np = std::use_facet(l);
-assert(np.grouping() == "\x7F");
+assert(np.grouping() == expected);
 }
 }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits