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, 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. http://codereview.chromium.org/1722003/diff/23001/24004#newcode254 src/bootstrapper.cc:254: bool make_prototype_enumerable = false); 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. http://codereview.chromium.org/1722003/diff/23001/24004#newcode418 src/bootstrapper.cc:418: // would not be used as constructors. would not -> can not 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); 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. http://codereview.chromium.org/1722003/diff/23001/24001 File test/sputnik/sputnik.status (right): http://codereview.chromium.org/1722003/diff/23001/24001#newcode56 test/sputnik/sputnik.status:56: Nice! http://codereview.chromium.org/1722003/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
