Message ID | AM4PR0701MB216262DA4723290D4B5B00EEE4530@AM4PR0701MB2162.eurprd07.prod.outlook.com |
---|---|
State | New |
Headers | show |
On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote: > Hi, > > as noted in PR bootstrap/71481, comment#4 currently > the trunk fails to bootstrap if the current language is > not english. A workaround is possible by setting LANG=C, > but OTOH it is rather easy to fix, by translating the string > in the assertion, as it is the only place that is affected by > the language setting. > > > Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8. > OK to commit? Sorry about the breakage. I believe I can approve this with my "libcpp"/"diagnostics" hats on, so LGTM. That said, should we hardcode LANG=C when running the selftests from gcc/Makefile.in? Dave
On 06/13/16 17:27, David Malcolm wrote: > On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote: >> Hi, >> >> as noted in PR bootstrap/71481, comment#4 currently >> the trunk fails to bootstrap if the current language is >> not english. A workaround is possible by setting LANG=C, >> but OTOH it is rather easy to fix, by translating the string >> in the assertion, as it is the only place that is affected by >> the language setting. >> >> >> Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8. >> OK to commit? > > Sorry about the breakage. > > I believe I can approve this with my "libcpp"/"diagnostics" hats on, so > LGTM. > Thanks. > That said, should we hardcode LANG=C when running the selftests from > gcc/Makefile.in? > Honestly, I am glad to see that there is some sort of unit test which runs in a different LANG setting than the rest of the testsuite. Because as this incident clearly shows, there _can_ be bugs that do not show up in the default locale. I would put the question this way: could it be possible to run also some tests in the testsuite with a LANG setting different from "C"? Bernd.
On Mon, Jun 13, 2016 at 03:39:21PM +0000, Bernd Edlinger wrote: > On 06/13/16 17:27, David Malcolm wrote: > > On Mon, 2016-06-13 at 14:41 +0000, Bernd Edlinger wrote: > >> Hi, > >> > >> as noted in PR bootstrap/71481, comment#4 currently > >> the trunk fails to bootstrap if the current language is > >> not english. A workaround is possible by setting LANG=C, > >> but OTOH it is rather easy to fix, by translating the string > >> in the assertion, as it is the only place that is affected by > >> the language setting. > >> > >> > >> Boot-strapped and reg-tested on trunk with LANG=de_DE.UTF-8. > >> OK to commit? > > > > Sorry about the breakage. > > > > I believe I can approve this with my "libcpp"/"diagnostics" hats on, so > > LGTM. > > > > Thanks. Please put PR bootstrap/71481 into the ChangeLog entry though. > > That said, should we hardcode LANG=C when running the selftests from > > gcc/Makefile.in? > > > > Honestly, I am glad to see that there is some sort of unit test which > runs in a different LANG setting than the rest of the testsuite. > Because as this incident clearly shows, there _can_ be bugs that do not > show up in the default locale. > > I would put the question this way: could it be possible to run also > some tests in the testsuite with a LANG setting different from "C"? I think running the s-selftest in C locale is a good idea, but maybe we should have some test in gcc.dg or where that would run -fself-tests in some other locale. I think right now we force LC_ALL=C for all tests, but perhaps /* { dg-set-compiler-env-var LC_ALL "something" } */ would work. But perhaps we'd need some tcl test for whether the locale is supported by the system. Jakub
2016-06-13 Bernd Edlinger <bernd.edlinger@hotmail.de> * input.c (test_builtins): Fix an assertion. Index: gcc/input.c =================================================================== --- gcc/input.c (Revision 237379) +++ gcc/input.c (Arbeitskopie) @@ -1210,7 +1210,7 @@ test_unknown_location () static void test_builtins () { - assert_loceq ("<built-in>", 0, 0, BUILTINS_LOCATION); + assert_loceq (_("<built-in>"), 0, 0, BUILTINS_LOCATION); ASSERT_PRED1 (is_location_from_builtin_token, BUILTINS_LOCATION); }