http://codereview.chromium.org/21450/diff/1/5 File src/jsregexp.cc (right):
http://codereview.chromium.org/21450/diff/1/5#newcode2273 Line 2273: pos->determines_perfectly = false; On 2009/02/18 15:20:44, Lasse Reichstein wrote: > Should this be set to false? If the remaining cases of a choice node are > actually determined perfectly, we should match them. > I.e., treat a cannot-match result as if it wasn't there at all. Determines perfectly indicates that this particular character position has been checked completely by the quick check. The implication is that if we get past the quick check then we don't need to look at this character position again. So 'false' would be wrong here. http://codereview.chromium.org/21450/diff/1/5#newcode2329 Line 2329: // succeed. On 2009/02/18 15:20:44, Lasse Reichstein wrote: > This comment should be changed to just "not yet handled". > It would be fairly easy to normalize the character class representation to be > non-negated and treat it like a positive class. I don't think that char classes like [^\u0000-ac-\uffff] are common enough to warrant spending time on, ever. http://codereview.chromium.org/21450/diff/1/5#newcode2677 Line 2677: return quick_check->positions(offset)->determines_perfectly; On 2009/02/18 15:20:44, Lasse Reichstein wrote: > Just checking: If we find that a position determines the branch perfectly, will > the position always be tested? Or is it possible that, e.g., both the first and > second character perfectly determines the branch, and only the first is actually > checked? See above. http://codereview.chromium.org/21450/diff/1/5#newcode2722 Line 2722: int* checked_up_to) { On 2009/02/18 15:20:44, Lasse Reichstein wrote: > This function is one big glob of special cases. > If the different passes has something in common, please refactor it to, e.g., a > template function or pass objects with virtual methods or something else that > avoids putting all the cases in one monolithic function. I've refactored a little more. It is now only 68 lines with 30 lines of comments to explain what it is doing. I feel that making it much smaller will either cause code duplication or unnecessary abstraction boilerplace elsewhere. http://codereview.chromium.org/21450 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
