[llvm-commits] [patch] ExceptionDemo patch for review

Eric Christopher echristo at apple.com
Thu Feb 4 18:44:31 PST 2010


On Feb 4, 2010, at 6:25 PM, Garrison Venn wrote:

> <ExceptionDemo.patch>

Looks pretty good. Couple of comments:

a) make sure your formatting of arguments matches the standard we've got, for example, createEntryBlockAlloca, handleLsda, etc. You do this a lot.
b) emittion -> emission, optimizatio->optimization
c) make sure everything is within 80 cols.
d) for (...) {
     }

In general, the code looks pretty good, the formatting is a bit wonky. Please don't check in any #if 0 code.  If you need to go ahead and comment something about it.  Little too much vertical spacing as well, no more than 1 line of space if you need some spacing, but definitely not in comments before code, e.g.:

// This is a comment
//
return something;

The comment in runExceptionThrow is a bit confusing, intptr_t btw is a long, I'm not sure what you're asking there.

Warnings about needing to compile with -m32 at the bottom of the code aren't very helpful. You could also make that a pointer size if that's what you want (and probably should).

Comments should be less commentary or pronoun based :)

Thanks for the work you've put in so far.

-eric



More information about the llvm-commits mailing list