Formal Review of std.regex (FReD)

[Dmitry Olshansky]

On 23.10.2011 11:28, Rainer Schuetze wrote:
>
>
> On 22.10.2011 21:05, Dmitry Olshansky wrote:
>> On 22.10.2011 20:56, Rainer Schuetze wrote:
>>> I haven't followed the discussion closely, and I cannot really comment
>>> on the core regex functionality, but I did actually use FReD as a
>>> replacement of a buggy std.regex once.
>>>
>>> In that case I wanted to have a lazily created static regex, but I did
>>> not find an official way to test whether a Regex has been initialized:
>>>
>>> static Regex!char re;
>>> if(!isInitializedRE(re))
>>> re = regex(r"^(.*)\(([0-9]+)\):(.*)$");
>>>
>>> So I implemented isInitializedRE() as "re.ir !is null" for std.regex and
>>> "re.captures() > 0" for fred, but that fails for being a "drop-in
>>> replacement".
>>
>> Coincidentally, you still can access re.ir property in this way.
>> Wow, I wonder how far with backwards compatibility I can go :)
>>
>> In both cases this relies on undocumented features.
>> Even now I can suggest a more portable and entirely generic way:
>>
>> if(re == Regex!(char).init)
>> {
>> //create re
>> }
>>
>> Though that risks doing more work then needed.
>>
>>>
>>> I think, both versions use implementation specifics, maybe there should
>>> be a documented way to test for being initialized.
>>>
>>
>> Definitely. How about adding an empty property + opCast to bool, with
>> that you'd get:
>> if(!re)
>> {
>> //create re
>> }
>>
>> and a bit more verbose:
>> if(re.empty)
>> {
>> //create re
>> }
>
> I think, this might be confused with normal usage, like "is this regex
> the empty string?" (Is "" a valid regex?). Maybe a more explicite
> "valid()" predicate would be fine.

"" is a valid regex that matches anywhere, with global flag it will 
match before any codepoint + once at end.
I'm not sure using 'valid' is good, it may mislead user to check it all 
over the place e.g.:
auto r = regex("blah");
if(r.valid())
...


>
>>
>>> I also noticed, that "auto match(R, RegEx)(R input, RegEx re);" appears
>>> twice in the documentation, same for "bmatch". I guess they should not
>>> appear together with the string versions.
>>>
>>
>> I gather that happens because there is another overload specifically for
>> C-T regexes. It's docs state just that, but lacking the template
>> constraint signatures are the same, so it indeed can cause some
>> confusion.
>> Maybe it would be better to just combine docs together, and leave one
>> overload undocumented.
>>
>
> As RegEx is a template argument here, it can stand for both Regex and
> StaticRegex, and that should be mentioned. Whether it has two different
> implementations is an implementation detail that does not need to bother
> the user.

OK, will do.

>
> If you want to keep the second entries, I'd recommend renaming the
> argument to StaticRegEx.


-- 
Dmitry Olshansky