http://codereview.chromium.org/1722003/diff/23001/24004
File src/bootstrapper.cc (right):

http://codereview.chromium.org/1722003/diff/23001/24004#newcode252
src/bootstrapper.cc:252: bool add_prototype_property,
On 2010/04/27 13:02:12, Erik Corry wrote:
Boolean arguments are very hard to read at the call sites.  I suggest
making
enums for this eg.
enum AddPrototypeProperty { DONT_ADD_PROTOTYPE_PROPERTY,
ADD_PROTOTYPE_PROPERTY};
I usually put the negative one first so they silently evaluate to
false in case
I miss somewhere where it is used as a boolean.

Done.

http://codereview.chromium.org/1722003/diff/23001/24004#newcode254
src/bootstrapper.cc:254: bool make_prototype_enumerable = false);
On 2010/04/27 13:02:12, Erik Corry wrote:
The style guide deprecates default arguments unless it is very obvious
what the
default should be.  I don't find this one obvious so I think we should
get rid
of the default.  I realize this was like this before you started
changing the
code.

Done.

http://codereview.chromium.org/1722003/diff/23001/24004#newcode418
src/bootstrapper.cc:418: // would not be used as constructors.
On 2010/04/27 13:02:12, Erik Corry wrote:
would not -> can not

Done.

http://codereview.chromium.org/1722003/diff/23001/24006
File src/objects.h (right):

http://codereview.chromium.org/1722003/diff/23001/24006#newcode2860
src/objects.h:2860: inline void set_function_without_prototype(bool
value);
On 2010/04/27 13:02:12, Erik Corry wrote:
It is better to name booleans in a positive way so that we don't end
up with
double negation:

set_function_without_prototype(false);  // Doesn't not have proto.

Done.

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

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

Reply via email to