Sorry for all the spelling mistakes.
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_( On 2009/08/18 12:36:23, Mads Ager wrote: > Shouldn't you initialize |has_only_this_property_assignments| as well? Done. 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(), On 2009/08/18 12:36:23, Mads Ager wrote: > Four space indent. Done. 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(); On 2009/08/18 12:36:23, Mads Ager wrote: > I think we normally place the operators on the previous line and we shouldn't we > use a four space indent? Done. 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. On 2009/08/18 12:36:23, Mads Ager wrote: > Calculate total number of properties described by map? Done. http://codereview.chromium.org/172088/diff/1003/41#newcode192 Line 192: #ifdef DEBUG On 2009/08/18 12:36:23, Mads Ager wrote: > Should this just be __ Assert(positive, "...")"? The Assert method in the macro > assembler only generates code if running with the --debug-code flag. Done. 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; On 2009/08/18 12:36:23, Mads Ager wrote: > Shouldn't this be a four space indent? Done. http://codereview.chromium.org/172088/diff/1003/40#newcode4804 Line 4804: kHasOnlyThisPropertyAssignments, On 2009/08/18 12:36:23, Mads Ager wrote: > Indentation is off here. This is an argument to set and not to > set_compiler_hints. Done. 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. On 2009/08/18 12:36:23, Mads Ager wrote: > Update comment. Done. 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. On 2009/08/18 12:36:23, Mads Ager wrote: > am -> an Done. 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 On 2009/08/18 12:36:23, Mads Ager wrote: > if the form -> of the form. Done. http://codereview.chromium.org/172088/diff/1003/50#newcode1463 Line 1463: // this.x = ...;, where x is a named property. It also findes whether a On 2009/08/18 12:36:23, Mads Ager wrote: > findes -> determines Done. http://codereview.chromium.org/172088/diff/1003/50#newcode1475 Line 1475: // Bail out if function already have had non this property assignment On 2009/08/18 12:36:23, Mads Ager wrote: > have had -> has Done. http://codereview.chromium.org/172088/diff/1003/50#newcode1491 Line 1491: // Returns whether only statements of the form thsi.x = ...; was encountered. On 2009/08/18 12:36:23, Mads Ager wrote: > thsi -> this Done. 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 On 2009/08/18 12:36:23, Mads Ager wrote: > thsi -> this Done. http://codereview.chromium.org/172088/diff/1003/50#newcode1579 Line 1579: assigned_constants_->Add(Factory::undefined_value()); On 2009/08/18 12:36:23, Mads Ager wrote: > Does this mean that 'this.x = undefined' is not recognized as a constant this > assignment? Yes, as undefined is a property on the global object and not a literal. The function f(undefined){this.x=undefined} is recognized as having an argument assignment - which should be correct. Also f(undefined){this.x=void 0} is not an assignment from a constant. http://codereview.chromium.org/172088/diff/1003/50#newcode1652 Line 1652: this_property_assignment_finder.only_this_property_assignments(), On 2009/08/18 12:36:23, Mads Ager wrote: > Four space indent. Done. 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; On 2009/08/18 12:36:23, Mads Ager wrote: > __ Assert? Done. http://codereview.chromium.org/172088 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
