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(); Not as such, no, but it's not a good property that using RemoveLast puts the data structure in a semi-inconsistent state. What I needed was actually to use last() after RemoveLast has been used and that didn't work. http://codereview.chromium.org/10759/diff/14/206#newcode315 Line 315: void AddTerm(RegExpTree* tree); You're right -- I've been using the term 'atom' incorrectly as meaning flat character sequences. I'll fix that. http://codereview.chromium.org/10759/diff/14/206#newcode368 Line 368: terms_.Add(text); If we're always using GetList anyway then why even use buffered lists? I'd rather not use GetList because in the (common) case where there is just one element if will allocate an unnecessary one-element list. We only ever need AppendToText for text nodes and passing the text node means that it doesn't have to expose that it happens to have been implemented using a list. I can easily imagine us changing the internal representation, for instance to avoid allocating a list when there is only one element. http://codereview.chromium.org/10759/diff/14/206#newcode389 Line 389: void RegExpBuilder::AddTerm(RegExpTree* term) { You don't always know that you're adding a character class. Consider "foo (?:[bar]) baz". Also, text nodes can themselves be added to other text nodes. http://codereview.chromium.org/10759/diff/14/206#newcode467 Line 467: ASSERT(last_added_ == ADD_ATOM); What about "foo (?:bar) baz"? http://codereview.chromium.org/10759 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
