This is the mail archive of the
mailing list for the GCC project.
Re: [patch] regex_traits implementation
- From: Jonathan Wakely <jwakely dot gcc at gmail dot com>
- To: Tim Shen <timshen91 at gmail dot com>
- Cc: "libstdc++" <libstdc++ at gcc dot gnu dot org>, gcc-patches <gcc-patches at gcc dot gnu dot org>, Stephen Webb <stephen dot webb at canonical dot com>
- Date: Tue, 2 Jul 2013 22:10:34 +0100
- Subject: Re: [patch] regex_traits implementation
- References: <CAPrifDmvwyXmOS4Wp1mo+ORnEk0o5=_dXbLimv9soahjcSuwAw at mail dot gmail dot com> <51CE1755 dot 1040401 at oracle dot com> <51CE1ED5 dot 6020104 at bregmasoft dot ca> <294c5ce0-0569-463f-b716-95abf2d2f848 at email dot android dot com> <CAGBC11ktNNzXhMLPuoxwyQ0o2QqEtq0S8Qi8YN3FMSsh5r_rZA at mail dot gmail dot com> <CAGBC11mJEnp70OdDdKr9BduhxXCrs9RUoN0TOXKhg7VSKnvQMQ at mail dot gmail dot com> <CAH6eHdShjBcZ+yF8O3+SOh222G5BQ5huMyE7-i0OhCTmbBhoeQ at mail dot gmail dot com> <CAPrifDnC--3U0KMW6DPZa_TCN=JAUrxLu3u-uTSihHPD_Hv_sg at mail dot gmail dot com>
On 2 July 2013 20:10, Tim Shen wrote:
> On Wed, Jul 3, 2013 at 1:11 AM, Jonathan Wakely <firstname.lastname@example.org> wrote:
>> I think the _RegexMask constructor should be explicit.
> I intentionally make _RegexMask act like an unsigned number. Since
> lookup_collatename is required to return a value compares equal to 0,
Oh, I hadn't noticed that requirement. I am inclined to consider that
a defect in the standard, it assumes an integral type, which bit-mask
types are not required to be. The standard should probably say it
compares equal to a value-initialized object instead, but given what
it says your choice to allow implicit conversions makes sense.
> I don't know if adding operater==(ctype_base __n) a good idea. It's a
> little bit elegant to make this implicit conver
Adding that operator wouldn't work for 0==x, only for x==0, so you'd
want a non-member, but let's keep the implicit conversion anyway.
>> Its parameters should be named __base and __extended, to conform to the libstdc++
>> coding guidelines.
Sorry for not being clear, the class data members should be _M_xxx but
the function parameters should be __xxx, they should not use the same
naming convention. That makes it clear when referring to a
(non-public) member rather than a local variable or function
parameter. The libstdc++ style guidelines are at
>> The operator functions take const parameters by value, is that a
>> conscious decision to prefer that to either non-const parameters or
>> pass by reference?
> It's intended.
> First, since sizeof(_RegexMask) is relatively small, a call-by-value
> call may save more time than pointer-based reference;
All the functions are inline, so it's unlikely to make much difference
in either case, both the reference binding and copying are likely to
be optimized away entirely except when using no optimization at all.
> Second, making it `const` enables more potential compiler optimizations.
As I said in my previous reply, I don't think this is true.
>> In theory we shouldn't need a special case for the "blank" character
>> class, I got that added to std::ctype_base via
>> http://cplusplus.github.io/LWG/lwg-defects.html#2019, but we don't
>> support it yet in libstdc++ (I forgot about it.)
> We don't have std::isblank yet; I add a comment for this.
Great, thanks. We should try to add isblank for GCC 4.9, so might be
able to revisit your regex_traits to use it before the release. If we
remove it, it might be better to remove the 1<<1 bit and keep 1<<0, so
it would make more sense to swap the values of _S_blank and _S_under.
I've just noticed that the try-catch block in transform_primary should
use the macros __try and __catch, which will allow the code to be used