| Message ID | ork0t9ila2.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | Split wchars tests from the normal variant | expand |
On 22/12/20 10:27 pm, Alexandre Oliva wrote: > This change extracts apart the wchar specific parts of character > conversion tests to allow conditonalizating these parts on actual > wchar support while applying the rest more generally. > > This turned out useful during our work on the libstdc++ support > for VxWorks, to expose the problematic areas more precisely. > > Regstrapped on x86_64-linux-gnu, and tested with -x-arm-wrs-vxworks7r2. > Ok to install? (dg-requires-wchars is added by another patch by > Corentin, that I posted a few minutes ago) > > While updating Corentin's patch for mainline, I brought over to the > split-out test even the preprocessor conditional that is present in the > current version of the test, but required/implied by dg-requires-wchars. > Maybe that's excessive. Maybe the whole patch is excessive, given that > conditional, but I didn't want to just drop it without asking for > others' thoughts. > > > from Corentin Gay <gay@adacore.com> > for libstdc++-v3/ChangeLog > > * testsuite/20_util/from_chars/1_neg.cc: Split wchar specific > part into... > * testsuite/20_util/from_chars/1_neg_wchar.cc: ... new file. > --- > libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc | 8 ----- > .../testsuite/20_util/from_chars/1_neg_wchar.cc | 35 ++++++++++++++++++++ > 2 files changed, 35 insertions(+), 8 deletions(-) > create mode 100644 libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc > > diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc > index 0d2fe2b3e6594..a84b0f5efb075 100644 > --- a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc > +++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc > @@ -23,14 +23,6 @@ > void > test01(const char* first, const char* last) > { > -#if _GLIBCXX_USE_WCHAR_T > - wchar_t wc; > -#else > - enum W { } wc; > -#endif > - std::from_chars(first, last, wc); // { dg-error "no matching" } > - std::from_chars(first, last, wc, 10); // { dg-error "no matching" } > - > char16_t c16; > std::from_chars(first, last, c16); // { dg-error "no matching" } > std::from_chars(first, last, c16, 10); // { dg-error "no matching" } > diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc > new file mode 100644 > index 0000000000000..2d736a28a2da7 > --- /dev/null > +++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc AFAIK _neg should be last. Using wchar_t_neg.cc should even make the dg-require-wchars useless here. > @@ -0,0 +1,35 @@ > +// Copyright (C) 2017-2018 Free Software Foundation, Inc. > +// > +// This file is part of the GNU ISO C++ Library. This library is free > +// software; you can redistribute it and/or modify it under the > +// terms of the GNU General Public License as published by the > +// Free Software Foundation; either version 3, or (at your option) > +// any later version. > + > +// This library is distributed in the hope that it will be useful, > +// but WITHOUT ANY WARRANTY; without even the implied warranty of > +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +// GNU General Public License for more details. > + > +// You should have received a copy of the GNU General Public License along > +// with this library; see the file COPYING3. If not see > +// <http://www.gnu.org/licenses/>. > + > +// { dg-options "-std=gnu++17" } > +// { dg-do compile { target c++17 } } > +// { dg-require-wchars {} } > + > +#include <charconv> > + > +void > +test01(const char* first, const char* last) > +{ > +#if _GLIBCXX_USE_WCHAR_T > + wchar_t wc; > +#else > + enum W { } wc; > +#endif Maybe you could simplify this as _GLIBCXX_USE_WCHAR_T is != 0. > + > + std::from_chars(first, last, wc); // { dg-error "no matching" } > + std::from_chars(first, last, wc, 10); // { dg-error "no matching" } > +} >
On Dec 28, 2020, François Dumont <frs.dumont@gmail.com> wrote: > On 22/12/20 10:27 pm, Alexandre Oliva wrote: >> While updating Corentin's patch for mainline, I brought over to the >> split-out test even the preprocessor conditional that is present in the >> current version of the test, but required/implied by dg-requires-wchars. >> Maybe that's excessive. >> +#if _GLIBCXX_USE_WCHAR_T >> + wchar_t wc; >> +#else >> + enum W { } wc; >> +#endif > Maybe you could simplify this as _GLIBCXX_USE_WCHAR_T is != 0. Yup, just as mentioned in the patch submission, in the paragraph quoted above.
On 22/12/20 18:27 -0300, Alexandre Oliva wrote: > >This change extracts apart the wchar specific parts of character >conversion tests to allow conditonalizating these parts on actual >wchar support while applying the rest more generally. > >This turned out useful during our work on the libstdc++ support >for VxWorks, to expose the problematic areas more precisely. > >Regstrapped on x86_64-linux-gnu, and tested with -x-arm-wrs-vxworks7r2. >Ok to install? (dg-requires-wchars is added by another patch by >Corentin, that I posted a few minutes ago) > >While updating Corentin's patch for mainline, I brought over to the >split-out test even the preprocessor conditional that is present in the >current version of the test, but required/implied by dg-requires-wchars. >Maybe that's excessive. Maybe the whole patch is excessive, given that >conditional, but I didn't want to just drop it without asking for >others' thoughts. I do think this is excessive. The point of the test is only to verify that calling from_chars with wchar_t gives an error. I don't think we need to make that conditional on whether wchar_t is supported or not. Adding a whole new test and checking the dg-requires... condition adds non-zero overhead to the testsuite. Following the theme of my other replies, maybe _GLIBCXX_USE_WCHAR_T isn't even the right thing to check here. We don't require any support for wchar_t in this test, we only require the type to be defined. Simple changing _GLIBCXX_USE_WCHAR_T to __SIZEOF_WCHAR_T__ seems like a better fix. That will mean that we use the type if it's defined, and not otherwise. We don't care if the library actually supports wchar_t specializations for std::char_traits etc. because we are expecting to get an error anyway.
On 28/12/20 19:39 +0100, François Dumont via Libstdc++ wrote: >On 22/12/20 10:27 pm, Alexandre Oliva wrote: >>This change extracts apart the wchar specific parts of character >>conversion tests to allow conditonalizating these parts on actual >>wchar support while applying the rest more generally. >> >>This turned out useful during our work on the libstdc++ support >>for VxWorks, to expose the problematic areas more precisely. >> >>Regstrapped on x86_64-linux-gnu, and tested with -x-arm-wrs-vxworks7r2. >>Ok to install? (dg-requires-wchars is added by another patch by >>Corentin, that I posted a few minutes ago) >> >>While updating Corentin's patch for mainline, I brought over to the >>split-out test even the preprocessor conditional that is present in the >>current version of the test, but required/implied by dg-requires-wchars. >>Maybe that's excessive. Maybe the whole patch is excessive, given that >>conditional, but I didn't want to just drop it without asking for >>others' thoughts. >> >> >>from Corentin Gay <gay@adacore.com> >>for libstdc++-v3/ChangeLog >> >> * testsuite/20_util/from_chars/1_neg.cc: Split wchar specific >> part into... >> * testsuite/20_util/from_chars/1_neg_wchar.cc: ... new file. >>--- >> libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc | 8 ----- >> .../testsuite/20_util/from_chars/1_neg_wchar.cc | 35 ++++++++++++++++++++ >> 2 files changed, 35 insertions(+), 8 deletions(-) >> create mode 100644 libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc >> >>diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc >>index 0d2fe2b3e6594..a84b0f5efb075 100644 >>--- a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc >>+++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc >>@@ -23,14 +23,6 @@ >> void >> test01(const char* first, const char* last) >> { >>-#if _GLIBCXX_USE_WCHAR_T >>- wchar_t wc; >>-#else >>- enum W { } wc; >>-#endif >>- std::from_chars(first, last, wc); // { dg-error "no matching" } >>- std::from_chars(first, last, wc, 10); // { dg-error "no matching" } >>- >> char16_t c16; >> std::from_chars(first, last, c16); // { dg-error "no matching" } >> std::from_chars(first, last, c16, 10); // { dg-error "no matching" } >>diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc >>new file mode 100644 >>index 0000000000000..2d736a28a2da7 >>--- /dev/null >>+++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc > >AFAIK _neg should be last. Yup. >Using wchar_t_neg.cc should even make the dg-require-wchars useless here. Indeed, but I don't think we want this change anyway.
diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc index 0d2fe2b3e6594..a84b0f5efb075 100644 --- a/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc +++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg.cc @@ -23,14 +23,6 @@ void test01(const char* first, const char* last) { -#if _GLIBCXX_USE_WCHAR_T - wchar_t wc; -#else - enum W { } wc; -#endif - std::from_chars(first, last, wc); // { dg-error "no matching" } - std::from_chars(first, last, wc, 10); // { dg-error "no matching" } - char16_t c16; std::from_chars(first, last, c16); // { dg-error "no matching" } std::from_chars(first, last, c16, 10); // { dg-error "no matching" } diff --git a/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc new file mode 100644 index 0000000000000..2d736a28a2da7 --- /dev/null +++ b/libstdc++-v3/testsuite/20_util/from_chars/1_neg_wchar.cc @@ -0,0 +1,35 @@ +// Copyright (C) 2017-2018 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// <http://www.gnu.org/licenses/>. + +// { dg-options "-std=gnu++17" } +// { dg-do compile { target c++17 } } +// { dg-require-wchars {} } + +#include <charconv> + +void +test01(const char* first, const char* last) +{ +#if _GLIBCXX_USE_WCHAR_T + wchar_t wc; +#else + enum W { } wc; +#endif + + std::from_chars(first, last, wc); // { dg-error "no matching" } + std::from_chars(first, last, wc, 10); // { dg-error "no matching" } +}