code review 1176043: re2: minor fixes

re2: minor fixes

[rsc, 2010-05-11 01:00:31]

Hello r (cc: re2-dev@googlegroups.com),

I'd like you to review this change.

[r2, 2010-05-11 01:01:35]

LGTM

On May 10, 2010, at 6:00 PM, rsc@golang.org wrote:

> Reviewers: r,
>
> Message:
> Hello r (cc: re2-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> re2: minor fixes
>
> Please review this at http://codereview.appspot.com/1176043/show
>
> Affected files:
>  M re2/parse.cc
>  M re2/testing/regexp_benchmark.cc
>
>
> Index: re2/parse.cc
> ===================================================================
> --- a/re2/parse.cc
> +++ b/re2/parse.cc
> @@ -158,6 +158,10 @@
>   DISALLOW_EVIL_CONSTRUCTORS(ParseState);
> };
>
> +// Pseudo-operators - only on parse stack.
> +const RegexpOp kLeftParen = static_cast<RegexpOp>(kMaxRegexpOp+1);
> +const RegexpOp kVerticalBar = static_cast<RegexpOp>(kMaxRegexpOp+2);
> +
> Regexp::ParseState::ParseState(ParseFlags flags,
>                                const StringPiece& whole_regexp,
>                                RegexpStatus* status)
> @@ -175,6 +179,8 @@
>   for (Regexp* re = stacktop_; re != NULL; re = next) {
>     next = re->down_;
>     re->down_ = NULL;
> +    if (re->op() == kLeftParen)
> +      delete re->name_;
>     re->Decref();
>   }
> }
> @@ -482,12 +488,6 @@
>   return true;
> }
>
> -// Pseudo-operators - only on parse stack.
> -
> -// Markers for left paren and pipe operators in the regexp stack.
> -const RegexpOp kLeftParen = static_cast<RegexpOp>(kMaxRegexpOp+1);
> -const RegexpOp kVerticalBar = static_cast<RegexpOp>(kMaxRegexpOp+2);
> -
> // Checks whether a particular regexp op is a marker.
> bool Regexp::ParseState::IsMarker(RegexpOp op) {
>   return op >= kLeftParen;
> @@ -1109,9 +1109,9 @@
> // The StringPiece must *NOT* be edited unless the call succeeds.
> UGroup* MaybeParsePerlCCEscape(StringPiece* s, Regexp::ParseFlags  
> parse_flags) {
>   if (!(parse_flags & Regexp::PerlClasses))
> -    return false;
> +    return NULL;
>   if (s->size() < 2 || (*s)[0] != '\\')
> -    return false;
> +    return NULL;
>   // Could use StringPieceToRune, but there aren't
>   // any non-ASCII Perl group names.
>   StringPiece name(s->begin(), 2);
> Index: re2/testing/regexp_benchmark.cc
> ===================================================================
> --- a/re2/testing/regexp_benchmark.cc
> +++ b/re2/testing/regexp_benchmark.cc
> @@ -95,7 +95,7 @@
>           static_cast<int>(sizeof(PCRE)),
>           static_cast<int>(sizeof(RE2)),
>           static_cast<int>(sizeof(Prog)),
> -          static_cast<int>(sizeof(Inst)));
> +          static_cast<int>(sizeof(Prog::Inst)));
> }
>
> // Regular expression implementation wrappers.
>
>

[rsc, 2010-05-11 01:02:26]

*** Submitted as http://code.google.com/p/re2/source/detail?r=7898bbcaf58f ***

re2: minor fixes

R=r
CC=re2-dev
http://codereview.appspot.com/1176043