Looks reasonable to me.
I look forward to seeing the analysis being used :)




http://codereview.chromium.org/10254/diff/1/3
File src/jsregexp.cc (right):

http://codereview.chromium.org/10254/diff/1/3#newcode672
Line 672: class SubAnalysisData: public AnalysisData {
How about AnalysisDataBuilder?
SubAnalysisData doesn't say anything about what it's good for, just that
it's a subclass.

http://codereview.chromium.org/10254/diff/1/3#newcode832
Line 832: visited_(false) { }
Why is it only ChoiceNode that has a visited field?
Other nodes might be the first to be visited twice.

http://codereview.chromium.org/10254/diff/1/3#newcode986
Line 986: fprintf(stderr, "--- %p ---\n", static_cast<void*>(this));
Debug-print?

http://codereview.chromium.org/10254/diff/1/6
File test/cctest/test-regexp.cc (right):

http://codereview.chromium.org/10254/diff/1/6#newcode610
Line 610: Execute("(a|b|c|\\w|\\s)", "", true);
Not *really* a test?

http://codereview.chromium.org/10254

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

Reply via email to