https://codereview.chromium.org/18587007/diff/7001/src/types.cc
File src/types.cc (right):

https://codereview.chromium.org/18587007/diff/7001/src/types.cc#newcode482
src/types.cc:482: char buffer[200];
200 seems fragile. I'd drop this method altogether and recurse directly
in the main method below. You can do that by eliminating the use of
SmallListPrinter, which seems more trouble than it's worth here.

https://codereview.chromium.org/18587007/diff/7001/src/types.cc#newcode499
src/types.cc:499: SimpleListPrinter printer(stream);
Can we put a "{...}" around the list?

https://codereview.chromium.org/18587007/diff/7001/src/types.cc#newcode516
src/types.cc:516: SimpleListPrinter printer(stream);
Can we put a "{...}" around the list?

https://codereview.chromium.org/18587007/diff/7001/src/types.h
File src/types.h (right):

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode142
src/types.h:142: static Type* type() { return from_bitset(k##type); }
This line needs indentation

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode210
src/types.h:210: // Declare const for all primitive types.
I think this comment is redundant (the "primitive" part also seems to be
misleading).

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode211
src/types.h:211: #define DECLARE_PRIMITIVE_TYPE(type, value) k##type =
(value),
Rename to DECLARE_TYPE

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode213
src/types.h:213: #undef DECLARE_OPCODE
Wrong macro name?

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode217
src/types.h:217: static const char* GetComposedName(int type) {
Can we move these helpers to the bottom?

https://codereview.chromium.org/18587007/diff/7001/src/types.h#newcode282
src/types.h:282: void PrintName(StringStream* stream);
It might be worth following the objects.h conventions here. That is:

- name this method TypePrint
- have overloads for printing to a FILE and to stdout
- make it public, but wrap it into an #ifdef OBJECT_PRINT

https://codereview.chromium.org/18587007/

--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.


Reply via email to