Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(253)

Issue 10750: * Update to RegExp parsing and AST. (Closed)

Created:
12 years, 1 month ago by Lasse Reichstein
Modified:
9 years, 6 months ago
CC:
v8-dev
Visibility:
Public.

Description

Better handling of captures. Ignores back-references to captures that can never be captured at that point. RegExpBackReference keeps link to the RegExpCapture. Fix for bad formatting.

Patch Set 1 #

Patch Set 2 : Addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+127 lines, -53 lines) Patch
M regexp2000/src/ast.h View 1 3 chunks +14 lines, -4 lines 0 comments Download
M regexp2000/src/jsregexp.cc View 1 chunk +4 lines, -3 lines 0 comments Download
M regexp2000/src/parser.cc View 1 16 chunks +74 lines, -16 lines 0 comments Download
M regexp2000/src/regexp-macro-assembler.h View 2 chunks +5 lines, -5 lines 0 comments Download
M regexp2000/src/regexp-macro-assembler.cc View 2 chunks +3 lines, -4 lines 0 comments Download
M regexp2000/src/regexp-macro-assembler-ia32.h View 1 2 chunks +4 lines, -1 line 0 comments Download
M regexp2000/src/regexp-macro-assembler-ia32.cc View 1 9 chunks +17 lines, -17 lines 0 comments Download
M regexp2000/test/cctest/test-regexp.cc View 1 2 chunks +6 lines, -3 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
Lasse Reichstein
Capture-reachability + backref links to capture. Please review.
12 years, 1 month ago (2008-11-14 13:43:17 UTC) #1
Christian Plesner Hansen
12 years, 1 month ago (2008-11-14 13:55:17 UTC) #2
Lgtm, with comments.

http://codereview.chromium.org/10750/diff/1/2
File regexp2000/src/ast.h (right):

http://codereview.chromium.org/10750/diff/1/2#newcode1346
Line 1346: CapturePermanentlyUnreachable } CaptureAvailability;
You should use all-caps names for enums.  Also, the formatting is odd.

http://codereview.chromium.org/10750/diff/1/4
File regexp2000/src/parser.cc (right):

http://codereview.chromium.org/10750/diff/1/4#newcode4043
Line 4043: captures_->Add(NULL);
This looks suspect.  Why do this instead of using a counter?

http://codereview.chromium.org/10750/diff/1/5
File regexp2000/src/regexp-macro-assembler-ia32.cc (right):

http://codereview.chromium.org/10750/diff/1/5#newcode286
Line 286: void RegExpMacroAssemblerIA32::GoTo(Label *to) {
Why not use the opportunity to put the * next to the type too.

http://codereview.chromium.org/10750/diff/1/5#newcode431
Line 431: void LoadConstantBufferAddress(Register reg, const ArraySlice<T>&
buffer) {
Why is this a reference and why is it const?

Powered by Google App Engine
This is Rietveld 408576698