LGTM, with small changes.

https://chromiumcodereview.appspot.com/9691040/diff/7013/src/full-codegen.h
File src/full-codegen.h (right):

https://chromiumcodereview.appspot.com/9691040/diff/7013/src/full-codegen.h#newcode820
src/full-codegen.h:820: Iterator lookup(Literal* literal) {
Nit: Shouldn't this be called 'find' and have the same interface?

On second thought, this wrapper class wouldn't even be necessary if the
HashMap itself was properly templatized and could work on arbitrary
types, not just pointers, like a real STL thingy. Then the accessor
struct could be flattened into the entry struct and needn't be allocated
separately.

Such a change might or might not blow up code size -- would be worth a
try (but not in this CL :) ).

https://chromiumcodereview.appspot.com/9691040/diff/7013/src/hashmap.h
File src/hashmap.h (right):

https://chromiumcodereview.appspot.com/9691040/diff/7013/src/hashmap.h#newcode301
src/hashmap.h:301: struct value_type {
Hm, value_type doesn't seem to fit. I'd call this entry_type, and name
the fields 'key' and 'value'.

https://chromiumcodereview.appspot.com/9691040/

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

Reply via email to