LGTM
http://codereview.chromium.org/172088/diff/1003/42 File src/ast.h (right): http://codereview.chromium.org/172088/diff/1003/42#newcode1243 Line 1243: has_only_simple_this_property_assignments_( Shouldn't you initialize |has_only_this_property_assignments| as well? http://codereview.chromium.org/172088/diff/1003/45 File src/codegen.cc (right): http://codereview.chromium.org/172088/diff/1003/45#newcode259 Line 259: lit->has_only_this_property_assignments(), Four space indent. http://codereview.chromium.org/172088/diff/1003/39 File src/heap.cc (right): http://codereview.chromium.org/172088/diff/1003/39#newcode2129 Line 2129: - map->inobject_properties(); I think we normally place the operators on the previous line and we shouldn't we use a four space indent? http://codereview.chromium.org/172088/diff/1003/41 File src/ia32/builtins-ia32.cc (right): http://codereview.chromium.org/172088/diff/1003/41#newcode183 Line 183: // Calculate total properties described map. Calculate total number of properties described by map? http://codereview.chromium.org/172088/diff/1003/41#newcode192 Line 192: #ifdef DEBUG Should this just be __ Assert(positive, "...")"? The Assert method in the macro assembler only generates code if running with the --debug-code flag. http://codereview.chromium.org/172088/diff/1003/40 File src/objects.cc (right): http://codereview.chromium.org/172088/diff/1003/40#newcode4785 Line 4785: expected_nof_properties() * kPointerSize; Shouldn't this be a four space indent? http://codereview.chromium.org/172088/diff/1003/40#newcode4804 Line 4804: kHasOnlyThisPropertyAssignments, Indentation is off here. This is an argument to set and not to set_compiler_hints. http://codereview.chromium.org/172088/diff/1003/44 File src/objects.h (right): http://codereview.chromium.org/172088/diff/1003/44#newcode2892 Line 2892: // The bytes at positions 2 and 3 are not in use at the moment. Update comment. http://codereview.chromium.org/172088/diff/1003/44#newcode3116 Line 3116: // this.x = y; where y is either a constant or refers to am argument. am -> an http://codereview.chromium.org/172088/diff/1003/50 File src/parser.cc (right): http://codereview.chromium.org/172088/diff/1003/50#newcode1462 Line 1462: // A ThisNamedPropertyAssigmentFinder finds and marks statements if the form if the form -> of the form. http://codereview.chromium.org/172088/diff/1003/50#newcode1463 Line 1463: // this.x = ...;, where x is a named property. It also findes whether a findes -> determines http://codereview.chromium.org/172088/diff/1003/50#newcode1475 Line 1475: // Bail out if function already have had non this property assignment have had -> has http://codereview.chromium.org/172088/diff/1003/50#newcode1491 Line 1491: // Returns whether only statements of the form thsi.x = ...; was encountered. thsi -> this http://codereview.chromium.org/172088/diff/1003/50#newcode1496 Line 1496: // Returns whether only statements of the form thsi.x = y; where y is either a thsi -> this http://codereview.chromium.org/172088/diff/1003/50#newcode1579 Line 1579: assigned_constants_->Add(Factory::undefined_value()); Does this mean that 'this.x = undefined' is not recognized as a constant this assignment? http://codereview.chromium.org/172088/diff/1003/50#newcode1652 Line 1652: this_property_assignment_finder.only_this_property_assignments(), Four space indent. http://codereview.chromium.org/172088/diff/1003/47 File src/x64/builtins-x64.cc (right): http://codereview.chromium.org/172088/diff/1003/47#newcode598 Line 598: Label assert, no_assert; __ Assert? http://codereview.chromium.org/172088 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
