https://chromiumcodereview.appspot.com/9691040/diff/2001/src/full-codegen.h File src/full-codegen.h (right):
https://chromiumcodereview.appspot.com/9691040/diff/2001/src/full-codegen.h#newcode816 src/full-codegen.h:816: class Accessors: public ZoneObject { On 2012/03/13 13:59:10, rossberg wrote:
This is just a simple pair, not abstracting anything, so a plain
struct seems
more appropriate.
I'm perfectly fine with this, and as Uncle Bob says: "The quasi-encapsulation of beans seems to make some OO purists feel better, but usually provides no other benefit." :-) https://chromiumcodereview.appspot.com/9691040/diff/2001/src/full-codegen.h#newcode833 src/full-codegen.h:833: Literal* first() const { return reinterpret_cast<Literal*>(key); } On 2012/03/13 13:59:10, rossberg wrote:
These can be static_cast's.
Done. https://chromiumcodereview.appspot.com/9691040/diff/2001/src/full-codegen.h#newcode853 src/full-codegen.h:853: return map_ != other.map_ || entry_ != other.entry_; On 2012/03/13 13:59:10, rossberg wrote:
Is the first condition necessary? I believe iterator comparison is
undefined
anyway if pointing into different containers.
In a first version, I left the first part out, but feared a comment saying "This is not correct for blablabla reasons.", so I added it. :-) Actually the first test is even totally removed by gcc in the loop condition for the iteration over the table, because it can statically see that it is always false. Removing it again... https://chromiumcodereview.appspot.com/9691040/diff/2001/src/ia32/full-codegen-ia32.cc File src/ia32/full-codegen-ia32.cc (right): https://chromiumcodereview.appspot.com/9691040/diff/2001/src/ia32/full-codegen-ia32.cc#newcode1416 src/ia32/full-codegen-ia32.cc:1416: __ push(Immediate(isolate()->factory()->null_value())); On 2012/03/13 13:59:10, rossberg wrote:
Not this change, but 'undefined' rather than 'null' is what is
typically used in
circumstances like this.
Nope, 'undefined' won't work here, we really need a value which can't be an accessor: 'undefined' can be one, but 'null' can not. https://chromiumcodereview.appspot.com/9691040/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
