[cfe-commits] [libcxx] r109512 - in /libcxx/trunk: include/regex src/regex.cpp test/re/re.alg/re.alg.search/basic.pass.cpp test/re/re.alg/re.alg.search/ecma.pass.cpp test/re/re.alg/re.alg.search/extended.pass.cpp test/re/re.const/re.synopt/syntax_option_type.pass.cpp

[Howard Hinnant]

On Jul 30, 2010, at 12:32 PM, Steven Watanabe wrote:

> AMDG
> 
> Howard Hinnant wrote:
>> Modified: libcxx/trunk/include/regex
>> URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/regex?rev=109512&r1=109511&r2=109512&view=diff
>> ==============================================================================
>> --- libcxx/trunk/include/regex (original)
>> +++ libcxx/trunk/include/regex Tue Jul 27 12:24:17 2010
>> @@ -747,12 +747,12 @@
>>     nosubs     = 1 << 1,
>>     optimize   = 1 << 2,
>>     collate    = 1 << 3,
>> -    ECMAScript = 1 << 4,
>> -    basic      = 1 << 5,
>> -    extended   = 1 << 6,
>> -    awk        = 1 << 7,
>> -    grep       = 1 << 8,
>> -    egrep      = 1 << 9
>> +    ECMAScript = 0,
>> +    basic      = 1 << 4,
>> +    extended   = 1 << 5,
>> +    awk        = 1 << 6,
>> +    grep       = 1 << 7,
>> +    egrep      = 1 << 8
>> };
>> 
> 
> I don't think this conforms to the requirements for bitmasks:
> "The value Y is set in the object X if the expression X & Y is nonzero."

Agreed.  But the regex interface is arguing differently:

   basic_regex(const charT* p, flag_type f = regex_constants::ECMAScript);

I tried a test case that looked like:

   std::regex re("[a[.ch.]z]", std::regex_constants::icase);

The constructor sent back an "unknown grammar" error.  There is no grammar called "icase" and this is the only way to enter this flag.  The problem is that the regex_constants::syntax_option_type enum mixes grammars and options into the same bitmask.  The other problem is that grammars shouldn't be bitmasks.  They should be enumerated types.  One can't specify more than one grammar.

One fix would be for the constructor to check the "grammar field" for zero and if found set it to (a non-zero) ECMAScript value.  Another fix is to go back to the committee and ask them to clean up this design.  Still another fix is to just make ECMAScript zero.  Fwiw, boost::regex made the "ECMAScript=0" choice.

-Howard