<table cellspacing="0" cellpadding="0" border="0" ><tr><td valign="top" style="font: inherit;"><div id="yiv1330559081"><table id="yiv1330559081bodyDrftID" class="yiv1330559081" border="0" cellpadding="0" cellspacing="0"><tbody><tr><td id="yiv1330559081drftMsgContent" style="font-family: inherit; font-style: inherit; font-variant: inherit; font-weight: inherit; font-size: inherit; line-height: inherit; font-size-adjust: inherit; font-stretch: inherit;">I cleaned up the indentation in the three patches and attached them. Please let me know if there are other issues. The patches should all apply cleanly with the latest revision. I will wait with submitting more patches until the MCJITSTreamer has been reviewed and checked in. <br><br>Thanks,<br>Jan<br><br>--- On <b>Sat, 8/21/10, Bruno Cardoso Lopes <i><bruno.cardoso@gmail.com></i></b> wrote:<br><blockquote style="border-left: 2px solid rgb(16, 16, 255); margin-left: 5px; padding-left: 5px;"><br>From:
Bruno Cardoso Lopes <bruno.cardoso@gmail.com><br>Subject: Re: [LLVMdev] MC-JIT Streamer 1/3<br>To: "Jan Sjodin" <jan_sjodin@yahoo.com><br>Cc: "Daniel Dunbar" <daniel@zuster.org>, "LLVM Developers Mailing List" <llvmdev@cs.uiuc.edu><br>Date: Saturday, August 21, 2010, 8:39 AM<br><br><div class="yiv1330559081plainMail">Hi Jan,<br><br>Just a random comment, the indentation in mem_raw_ostream::write_impl<br>and mem_raw_ostream::current_pos should also be 2 spaces instead of 4!<br><br>On Fri, Aug 20, 2010 at 12:38 PM, Jan Sjodin <<a rel="nofollow">jan_sjodin@yahoo.com</a>> wrote:<br>> I was delayed creating the smaller patches, but finally I had some time to put the first set together. There are three small patches, the first two are classes the MCJITStreamer uses, and the last patch is the MCJITStreamer class itself.<br>><br>> - Jan<br>><br>> --- On Sun, 8/1/10, Daniel Dunbar <<a
rel="nofollow">daniel@zuster.org</a>> wrote:<br>><br>>> From: Daniel Dunbar <<a rel="nofollow">daniel@zuster.org</a>><br>>> Subject: Re: [LLVMdev] MC-JIT Patches 2/3<br>>> To: "Jan Sjodin" <<a rel="nofollow">jan_sjodin@yahoo.com</a>><br>>> Cc: "LLVM Developers Mailing List" <<a rel="nofollow">llvmdev@cs.uiuc.edu</a>><br>>> Date: Sunday, August 1, 2010, 6:58 PM<br>>> Hi Jan,<br>>><br>>> I would rather not work with a patch this large. Can you<br>>> pull out the<br>>> addition of the MCJITStreamer into its own patch, and we<br>>> can iterate<br>>> on getting that in as a single commit? I realize it won't<br>>> work or do<br>>> anything useful, but I can't deal with reviewing patches<br>>> this large.<br>>> The main thing I am concerned about is getting the basic<br>>> design of how<br>>> the streamer and the assembler and the
object writer should<br>>> interact.<br>>><br>>> I also have a few general style comments:<br>>> 1. Please use:<br>>> --<br>>> case A: {<br>>> ...<br>>> }<br>>> --<br>>> instead
of:<br>>> --<br>>> case A:<br>>> {<br>>> ...<br>>> }<br>>> --<br>>><br>>> 2. Please use complete sentences in comments, including<br>>> leading<br>>> capitalization and punctuation. Also, please put a space<br>>> after the<br>>> comment marker ('/// Class', not '///class').<br>>><br>>> 3. Use doxygen comments when appropriate (for example, on<br>>> methods,<br>>> instance variables).<br>>><br>>> I have various comments on MCJITStreamer itself, but I will<br>>> hold them<br>>> until we have an individual patch for that class.<br>>><br>>> - Daniel<br>> _______________________________________________<br>> LLVM Developers mailing list<br>> <a rel="nofollow">LLVMdev@cs.uiuc.edu</a> <a rel="nofollow" target="_blank"
href="http://llvm.cs.uiuc.edu">http://llvm.cs.uiuc.edu</a><br>> <a rel="nofollow" target="_blank" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>><br>><br><br><br><br>-- <br>Bruno Cardoso Lopes<br><a rel="nofollow" target="_blank" href="http://www.brunocardoso.cc">http://www.brunocardoso.cc</a><br></div></blockquote></td></tr></tbody></table></div></td></tr></table>