[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