I think it's almost LGTM
http://codereview.chromium.org/8803024/diff/1001/build/common.gypi File build/common.gypi (right): http://codereview.chromium.org/8803024/diff/1001/build/common.gypi#newcode84 build/common.gypi:84: 'v8_postmortem_support': 'false', should not it use % at the end of the name? http://code.google.com/p/gyp/wiki/InputFormatReference#Providing_Default_Values_for_Variables_(%) a small comment about the nature of the flag above it would be also helpful. http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py File tools/gen-postmortem-metadata.py (right): http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode4 tools/gen-postmortem-metadata.py:4: # Copyright 2006-2008 the V8 project authors. All rights reserved. 2012 http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode56 tools/gen-postmortem-metadata.py:56: { 'name': 'FirstNonstringType', 'value': 'FIRST_NONSTRING_TYPE' }, I seems you can just list what you want to export and come up with some meaningful mangling scheme to avoid duplication and stuff. (just thinking loud) http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode208 tools/gen-postmortem-metadata.py:208: # Did you consider using TYPE_CHECKER macro as a feedback for this loop? http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode244 tools/gen-postmortem-metadata.py:244: # Mapping string types is more complicated. Both types and I think it might be fine to hard wire some instance type names to class names instead of trying to guess them in such a convoluted way. (just a thought) http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode247 tools/gen-postmortem-metadata.py:247: # In the simplest case, both of these are explicit in both names, long line. http://codereview.chromium.org/8803024/diff/1001/tools/gen-postmortem-metadata.py#newcode291 tools/gen-postmortem-metadata.py:291: typeclasses[type] = cctype; Maybe tools should have a whitelist and complain to stdout when it fails to map class into instance type? This way we can detect if tool will ever go out of sync with V8 source base. http://codereview.chromium.org/8803024/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
