| Message ID | ydd7gfwpcw5.fsf@lokon.CeBiTec.Uni-Bielefeld.DE |
|---|---|
| State | New |
| Headers | show |
Hi, >I wasn't certain about the right convention. The following patch >allowed bootstrap to finish on i386-pc-solaris2.10 and >x86_64-unknown-linux-gnu. I'll commit the patch once Solaris testing >has finished. Thanks a lot. >E.g. the first one is > >FAIL: 28_regex/algorithms/regex_match/basic/string_01.cc (test for >excess errors) >Excess errors: >/vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/basic/string_01.cc:34:47: >error: call of overloaded 'basic_regex(const char [8], const >flag_type&)' is ambiguous > >The other failures are similar. Tim, please address this ASAP, otherwise we have to revert the whole thing. Paolo
> Hi Paolo, > > >>This is already documented: > >> > >>http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#c > oding_style.bad_identifiers > > > > Indeed. As a simple to remember rule never use single underscore + > single > > uppercase. Usually we add a p, like _Cp, etc. Plenty of examples > > everywhere. At the moment I can't do the search&replace, any such patch > is > > preapproved though. > > I wasn't certain about the right convention. The following patch > allowed bootstrap to finish on i386-pc-solaris2.10 and > x86_64-unknown-linux-gnu. I'll commit the patch once Solaris testing > has finished. I can confirm that this patch fixes the arm-none-eabi build as well. Thanks, Kyrill
On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Tim, please address this ASAP, otherwise we have to revert the whole thing.
I'm trying to reproduce the compilation failures.
On Thu, Aug 8, 2013 at 10:04 PM, Tim Shen <timshen91@gmail.com> wrote: > On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Tim, please address this ASAP, otherwise we have to revert the whole thing. > > I'm trying to reproduce the compilation failures. There's a typedef in regex_constants.h: `typedef unsigned int syntax_option_type;` Which is a little bit naive. It possibly conflicts with size_t under i386 when overloading. I'm trying to change it to a bitset. Or is there any better solution?
Hi, >There's a typedef in regex_constants.h: > >`typedef unsigned int syntax_option_type;` > >Which is a little bit naive. It possibly conflicts with size_t under >i386 when overloading. I'm trying to change it to a bitset. Or is >there any better solution? In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. Do you really have to do overloading between size_t and that type? Or maybe you can use a type *smaller* than unsigned int. Paolo
On Thu, Aug 8, 2013 at 11:14 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. Do you really have to do overloading between size_t and that type? Or maybe you can use a type *smaller* than unsigned int.
The n3376 standard specified the following constructor:
...
explicit basic_regex(const charT* p, flag_type f = regex_constants::ECMAScript);
basic_regex(const charT* p, size_t len, flag_type f =
regex_constants::ECMAScript);
...
So flag_type shall not be the same as size_t. I don't know if when I
switch flag_type from unsigned int to, say, unsigned short, conflicts
will appear in 16bit archtectures.
Paolo Carlini <paolo.carlini@oracle.com> writes: > In my humble opinion involving the whole std::bitset container for a > syntax option is way overkill. It's already used for match_flag_type anyway. Andreas.
Hi, >So flag_type shall not be the same as size_t. I don't know if when I >switch flag_type from unsigned int to, say, unsigned short, conflicts >will appear in 16bit archtectures. Well 16-bit archs aren't really supported these days, as long as the c++ runtime is concerned. Thus if unsigned short works let's go with it and move on. In any case, we don't have to make a super final decision, but breaking 32 bit is bad. In general, I would recommend being careful with adding templated containers, remember that eventually we want to have a lot of code in *.cc files and instantiations will be a big big annoyance. Paolo
Hi, >> In my humble opinion involving the whole std::bitset container for a >> syntax option is way overkill. > >It's already used for match_flag_type anyway. Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere. Paolo Paolo
On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere.
So here's the change. It's under testing now, but could took several
hours. If someone has a faster machine, please tell the result :)
As a simple test, `g++ -m32 test.cc` didn't fail.
On 08/08/2013 06:51 PM, Tim Shen wrote: > On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere. > So here's the change. It's under testing now, but could took several > hours. If someone has a faster machine, please tell the result :) > > As a simple test, `g++ -m32 test.cc` didn't fail. Thanks! Paolo.
On Fri, Aug 9, 2013 at 12:51 AM, Tim Shen <timshen91@gmail.com> wrote: > So here's the change. It's under testing now, but could took several > hours. If someone has a faster machine, please tell the result :) Unfortuantely using `unsigned int => unsigned short` cannot work. Instead, I create a enum and do some operator overloadings. This patch passed bootstrap and all tests under the configuration under x86_64: --with-arch=corei7 --with-cpu=corei7 --prefix=/usr/local --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --with-fpmath=sse --enable-languages=c++
Hi, On 08/09/2013 07:08 AM, Tim Shen wrote: > On Fri, Aug 9, 2013 at 12:51 AM, Tim Shen <timshen91@gmail.com> wrote: >> So here's the change. It's under testing now, but could took several >> hours. If someone has a faster machine, please tell the result :) > Unfortuantely using `unsigned int => unsigned short` cannot work. > Instead, I create a enum and do some operator overloadings. > > This patch passed bootstrap and all tests under the configuration under x86_64: > > --with-arch=corei7 --with-cpu=corei7 --prefix=/usr/local > --enable-clocale=gnu --with-system-zlib --enable-shared > --with-demangler-in-ld --with-fpmath=sse --enable-languages=c++ Yes, if, as it should, it works on -m32 too, let's go with this. By the way, you didn't say how exactly you are testing?!? Because just make check doesn't test -m32. I use, something like: make -k check-target-libstdc++-v3 RUNTESTFLAGS="--target_board='unix/\{-m32,-m64\}'" for sure there are other ways to obtain the same result. Paolo.
On Fri, Aug 9, 2013 at 2:59 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Yes, if, as it should, it works on -m32 too, let's go with this. By the way, > you didn't say how exactly you are testing?!? Because just make check > doesn't test -m32. I use, something like: > > make -k check-target-libstdc++-v3 > RUNTESTFLAGS="--target_board='unix/\{-m32,-m64\}'" > > for sure there are other ways to obtain the same result. Committed. I entered `gcc-build/x86_64-unknown-linux-gnu/32/libstdc++-v3` then run `make check`. I see -m32 in test log.
Tim Shen <timshen91@gmail.com> writes: > On Fri, Aug 9, 2013 at 2:59 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote: >> Yes, if, as it should, it works on -m32 too, let's go with this. By the way, >> you didn't say how exactly you are testing?!? Because just make check >> doesn't test -m32. I use, something like: >> >> make -k check-target-libstdc++-v3 >> RUNTESTFLAGS="--target_board='unix/\{-m32,-m64\}'" >> >> for sure there are other ways to obtain the same result. > > Committed. > > I entered `gcc-build/x86_64-unknown-linux-gnu/32/libstdc++-v3` then > run `make check`. I see -m32 in test log. As expected, i386-pc-solaris2.10 testsuite results are back to normal now. Thanks. Rainer
diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h --- a/libstdc++-v3/include/bits/regex.h +++ b/libstdc++-v3/include/bits/regex.h @@ -995,18 +995,18 @@ namespace std _GLIBCXX_VISIBILITY(defaul const basic_regex<_CharT, _TraitsT>&, regex_constants::match_flag_type); - template<typename _B, typename _A, typename _C, typename _R> + template<typename _Bp, typename _Ap, typename _Cp, typename _Rp> friend bool - regex_match(_B, _B, - match_results<_B, _A>&, - const basic_regex<_C, _R>&, + regex_match(_Bp, _Bp, + match_results<_Bp, _Ap>&, + const basic_regex<_Cp, _Rp>&, regex_constants::match_flag_type); - template<typename _B, typename _A, typename _C, typename _R> + template<typename _Bp, typename _Ap, typename _Cp, typename _Rp> friend bool - regex_search(_B, _B, - match_results<_B, _A>&, - const basic_regex<_C, _R>&, + regex_search(_Bp, _Bp, + match_results<_Bp, _Ap>&, + const basic_regex<_Cp, _Rp>&, regex_constants::match_flag_type); flag_type _M_flags; @@ -2111,16 +2111,16 @@ namespace std _GLIBCXX_VISIBILITY(defaul template<typename, typename, typename, typename> friend class __detail::_BFSExecutor; - template<typename _B, typename _A, typename _Ch_type, typename _Rx_traits> + template<typename _Bp, typename _Ap, typename _Ch_type, typename _Rx_traits> friend bool - regex_match(_B, _B, match_results<_B, _A>&, + regex_match(_Bp, _Bp, match_results<_Bp, _Ap>&, const basic_regex<_Ch_type, _Rx_traits>&, regex_constants::match_flag_type); - template<typename _B, typename _A, typename _Ch_type, typename _Rx_traits> + template<typename _Bp, typename _Ap, typename _Ch_type, typename _Rx_traits> friend bool - regex_search(_B, _B, match_results<_B, _A>&, + regex_search(_Bp, _Bp, match_results<_Bp, _Ap>&, const basic_regex<_Ch_type, _Rx_traits>&, regex_constants::match_flag_type);