User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2a1pre) Gecko/20090302 Minefield/3.2a1pre Build Identifier: Most of these bugs are fairly trivial, so this bug will act as a catchall. The list follows, in order of modifications in the patch. * add a TYPE_OFFSET_BASETYPE function --- a direct transliteration of the gcc macro * add support for printing OFFSET_TYPE nodes (utilizing TYPE_OFFSET_BASETYPE) * add support for printing RESULT_DECLs * give more information about instruction nodes we don't know how to print * add an isEmpty() method to the Set ADT * handle RESULT_DECL, RESX_EXPR, and LABEL_EXPR in isn_defs() and isn_uses() * allow consumers of ESP.Analysis to provide initial "blames" for property variables * add an option to do_dehydra_dump that prints the indentation level along with the object's properties (not enabled by default by dehydra_dump()) Reproducible: Always

Pre-review comments: - I see what looks like a few nugatory whitespace changes. Are those fixes, or something that should be removed from the patch? - return statements are weird in GIMPLE. I believe they can (a) contain nothing ('return;'), (b) wrap a GIMPLE_MODIFY_STMT ('return x;' -> R.0124 := x'), and occasionally (c) wrap a simple expression. It's not clear why both (b) and (c) exist. Anyway, that's the source of the pretty-printing code that surprised you. Otherwise, looks pretty good.

Attempt to address comment 2. I saw one unnecessary whitespace delta that I removed. BTW, these bugfixes were present in the first patch, but omitted in comment 0: * fixed bug in isn_defs() * use 'Date.now()' instead of 'new Date()' when checking for esp timeout I think the patch is ready for review.

Comment on attachment 366649 [details] [diff] [review] Bugfixes, v2 Some minor issues here. In the future can you set the review? to me/dmandelin when you are ready for review. Otherwise we might miss your comment. >--- a/libs/gcc_print.js >+++ b/libs/gcc_print.js >@@ -64,18 +64,17 @@ function type_string(type) { > return type_string(TREE_TYPE(type)) + infix + '[]'; > } else if (code == REFERENCE_TYPE) { > return type_string(TREE_TYPE(type)) + infix + '&'; > } else if (code == FUNCTION_TYPE) { > return type_string(TREE_TYPE(type)) + ' (*)(' + type_args_string(type) + ')'; > } else if (code == METHOD_TYPE) { > return type_string(TREE_TYPE(type)) + ' (?::*)(' + type_args_string(type) + ')'; > } else if (code == OFFSET_TYPE) { >- return type_string(TREE_TYPE(type)) + " " >- + type_string(TYPE_OFFSET_BASETYPE(type))+ "::*" >+ return 'offsetof('+ type_string(TYPE_OFFSET_BASETYPE(type)) +', '+ type_string(TREE_TYPE (type)); > } While changing this, would you mind making that if/else chain into a switch? > >+ /** True if this Set has no elements. */ >+ Set.prototype.isEmpty = function() { >+ for (let k in this.items ()) { >+ return false; >+ } >+ return true; >+ }; >+ I think that should just be return this.map.isEmpty() Dave can comment on the rest

Thanks! Missed the "review" flag. Requested tglek for review; please advise if it'd be better for dmandelin to review this.

Comment on attachment 366768 [details] [diff] [review] Bugfixes, v3 parts I know look good to me.

Comment on attachment 366768 [details] [diff] [review] Bugfixes, v3 Cleaning up ancient review requests.

Dehydra and treehydra are no longer maintained by Mozilla.