Type::GetName() for printing types in debugger

Type::GetName(...) for inspecting Types in the debugger

BUG=
R=rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15586

[rossberg, 2013-07-03 14:52:54]

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

https://codereview.chromium.org/18587007/diff/1/src/types.cc#newcode501
src/types.cc:501: for (int i = 0; i < unioned->length(); ++i) {
Note that for a union, at most one part is not a constant or class. Since you do
not print any information to distinguish either of those anyway, it might make
sense to just print a summary NumClasses and NumConstants for a union, plus the
bitset part if present.

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

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode117
src/types.h:117: V(Oddball,         kBoolean | kNull | kUndefined)              
           \
Nit: Maybe insert an empty line before this one.

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode124
src/types.h:124: V(Object,     kUndetectable | kArray | kFunction | kRegExp |
kOtherObject) \
Aha, cheating with indentation!

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode133
src/types.h:133: static Type* None() { return from_bitset(kNone); }
Can't you generate all these functions from the macro now?

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode231
src/types.h:231: PRIMITIVE_TYPE_LIST(DECLARE_PRIMITIVE_TYPE)
Nit: maybe indent this by 2

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode233
src/types.h:233: kUnusedEOL = 0
What's that for?

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode237
src/types.h:237: switch (as_bitset()) {
Yeah, that does not fly. Any combination of primitive types represents a valid
(union) type, so you must be able to print all of them.

[oliv, 2013-07-05 12:29:06]

PTAL

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

https://codereview.chromium.org/18587007/diff/1/src/types.cc#newcode501
src/types.cc:501: for (int i = 0; i < unioned->length(); ++i) {
On 2013/07/03 14:52:54, rossberg wrote:
> Note that for a union, at most one part is not a constant 
> or class. Since you do
> not print any information to distinguish either of those 
> anyway, it might make
> sense to just print a summary NumClasses and NumConstants 
> for a union, plus the
> bitset part if present.

As discussed offline, i now print the addresses of those elements, so its again
usefull to keep them in the output

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

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode231
src/types.h:231: PRIMITIVE_TYPE_LIST(DECLARE_PRIMITIVE_TYPE)
On 2013/07/03 14:52:54, rossberg wrote:
> Nit: maybe indent this by 2
i'd rather not, since it looks very strange if the defined macro goes over
multiple lines.

https://codereview.chromium.org/18587007/diff/1/src/types.h#newcode233
src/types.h:233: kUnusedEOL = 0
On 2013/07/03 14:52:54, rossberg wrote:
> What's that for?
you cannot end the enum definition with a comma...

[rossberg, 2013-07-08 11:20:05]

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

[oliv, 2013-07-10 10:25:41]

PTAL

[rossberg, 2013-07-10 10:32:08]

LGTM with comments

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

https://codereview.chromium.org/18587007/diff/23001/src/types.cc#newcode509
src/types.cc:509: PrintF(out, "Constant(0x%x)", (unsigned)(*as_constant()));
Is there a specific reason you are not using %p (here and below)?

https://codereview.chromium.org/18587007/diff/23001/src/types.cc#newcode517
src/types.cc:517: if (i>0) PrintF(out, ",");
Nit: spaces around >

[oliv, 2013-07-10 11:19:43]

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

https://codereview.chromium.org/18587007/diff/23001/src/types.cc#newcode509
src/types.cc:509: PrintF(out, "Constant(0x%x)", (unsigned)(*as_constant()));
On 2013/07/10 10:32:08, rossberg wrote:
> Is there a specific reason you are not using %p (here and below)?

none except didn't remember the parameter

[oliv, 2013-07-10 11:20:06]

Committed patchset #5 manually as r15586 (presubmit successful).

[rossberg, 2013-07-10 12:01:52]

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

https://codereview.chromium.org/18587007/diff/30001/src/types.cc#newcode518
src/types.cc:518: PrintF(out, "Constant(%p)",
reinterpret_cast<void*>(*as_constant()));
This should be a static_cast ;)