[v8-dev] Re: Fix a bug that prevents constants from overwriting function values in object ... (issue4004006)

[ager]

This is definitely getting there. One more iteration and I think it's  
ready. :)

http://codereview.chromium.org/4004006/diff/18001/19002
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/4004006/diff/18001/19002#newcode1212
src/arm/full-codegen-arm.cc:1212: VisitForStackValue(value);
You should always visit and then decide whether to store the value to
not loose side-effects of evaluating the value.

http://codereview.chromium.org/4004006/diff/18001/19003
File src/ast.cc (right):

http://codereview.chromium.org/4004006/diff/18001/19003#newcode184
src/ast.cc:184: bool IsEqual(void* first, void* second) {
I think you should split this into two functions. StringEquals and
SmiEquals. Then you don't need the conditionals here.

http://codereview.chromium.org/4004006/diff/18001/19003#newcode204
src/ast.cc:204: for (int i = this->properties()->length()-1; i >= 0;
i--) {
Space before and after '-'. Consider pulling out the length extraction
from the for loop.

http://codereview.chromium.org/4004006/diff/18001/19003#newcode238
src/ast.cc:238: // If the key of a computed property is in the table,
mark the property.
mark the property -> do not emit a store for the property.

http://codereview.chromium.org/4004006/diff/18001/19007
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/4004006/diff/18001/19007#newcode43
src/ia32/codegen-ia32.cc:43: #include "codegen.h"
Remove?

http://codereview.chromium.org/4004006/diff/18001/19007#newcode5579
src/ia32/codegen-ia32.cc:5579: // Duplicate the object as the IC
receiver.
Two spaces inserted by accident I think. :)

http://codereview.chromium.org/4004006/diff/18001/19007#newcode5592
src/ia32/codegen-ia32.cc:5592: frame_->Pop();
Use Drop as suggested by Kevin.

http://codereview.chromium.org/4004006/diff/18001/19007#newcode5606
src/ia32/codegen-ia32.cc:5606: // Ignore the result.
Indent by two more spaces. I would put the comment before the runtime
call.

http://codereview.chromium.org/4004006/diff/18001/19007#newcode5608
src/ia32/codegen-ia32.cc:5608: frame_->Pop();
Use Drop.

http://codereview.chromium.org/4004006/diff/18001/19008
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/4004006/diff/18001/19008#newcode39
src/ia32/full-codegen-ia32.cc:39: #include "codegen.h"
Not needed?

http://codereview.chromium.org/4004006/show

--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev