http://codereview.chromium.org/10759/diff/14/206
File src/parser.cc (right):

http://codereview.chromium.org/10759/diff/14/206#newcode260
Line 260: last_ = list_->RemoveLast();
Ah, so there was a need :)

http://codereview.chromium.org/10759/diff/14/206#newcode368
Line 368: terms_.Add(text);
The current uses all have special cases for one element, where they just
use last(). If there are more than one element, they use GetList() to
get all the elements as a list, and pass it to the constructors that
already happened to expect a list (RegExpDisjunction,
RegExpAlternatives).

RegExpText seems to follow the same pattern: If there is only one
element, just return that, otherwise create a wrapper RegExpText to
contain the list of text elements. If RegExpText had a constructor
taking a pointer to a list, it could choose to use that instead of
creating a new list.
However, I can see that adding one RegExpText node to another might want
to flatten the resulting tree, which leads to different requirements.

http://codereview.chromium.org/10759/diff/14/206#newcode389
Line 389: void RegExpBuilder::AddTerm(RegExpTree* term) {
I can see the point. If the result of a sub-parse is a single text node,
then it might be recognized as such an added to text_ instead of terms_.
So I would use AddText to add it, and any character classes, together
with setting last_ to ADD_TEXT.

http://codereview.chromium.org/10759/diff/14/206#newcode467
Line 467: ASSERT(last_added_ == ADD_ATOM);
Indeed. It would have to be ADD_TEXT to capture that.

http://codereview.chromium.org/10759/diff/14/206#newcode3640
Line 3640: builder.AddTerm(atom);
At this point we could check whether the atom is a text node, and use a
specialized AddText instead if it is.
We can't recognize assertions and use AddAssertion, because (?:\b) is an
atom and can be quantified, even if it's ridiculous :(

http://codereview.chromium.org/10759

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to