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

[Garrison Venn]

Hi Eric,

Thanks for your time. As much as I can tell, the attached patch has your recommended changes implemented.
Do you have further changes you want me to make before check in?

Thanks in advance

Garrison

On Feb 4, 2010, at 21:44, Eric Christopher wrote:

> 
> 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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: ExceptionDemo.patch
Type: application/octet-stream
Size: 77997 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100205/b18f3aed/attachment.obj>