[LLVMdev] MC-JIT Streamer 1/3

Bruno Cardoso Lopes bruno.cardoso at gmail.com
Sat Aug 21 06:39:21 PDT 2010


Hi Jan,

Just a random comment, the indentation in mem_raw_ostream::write_impl
and mem_raw_ostream::current_pos should also be 2 spaces instead of 4!

On Fri, Aug 20, 2010 at 12:38 PM, Jan Sjodin <jan_sjodin at yahoo.com> wrote:
> 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.
>
> - Jan
>
> --- On Sun, 8/1/10, Daniel Dunbar <daniel at zuster.org> wrote:
>
>> From: Daniel Dunbar <daniel at zuster.org>
>> Subject: Re: [LLVMdev] MC-JIT Patches 2/3
>> To: "Jan Sjodin" <jan_sjodin at yahoo.com>
>> Cc: "LLVM Developers Mailing List" <llvmdev at cs.uiuc.edu>
>> Date: Sunday, August 1, 2010, 6:58 PM
>> Hi Jan,
>>
>> I would rather not work with a patch this large. Can you
>> pull out the
>> addition of the MCJITStreamer into its own patch, and we
>> can iterate
>> on getting that in as a single commit? I realize it won't
>> work or do
>> anything useful, but I can't deal with reviewing patches
>> this large.
>> The main thing I am concerned about is getting the basic
>> design of how
>> the streamer and the assembler and the object writer should
>> interact.
>>
>> I also have a few general style comments:
>>  1. Please use:
>> --
>> case A: {
>>   ...
>> }
>> --
>> instead of:
>> --
>> case A:
>>   {
>>     ...
>>   }
>> --
>>
>>  2. Please use complete sentences in comments, including
>> leading
>> capitalization and punctuation. Also, please put a space
>> after the
>> comment marker ('/// Class', not '///class').
>>
>>  3. Use doxygen comments when appropriate (for example, on
>> methods,
>> instance variables).
>>
>> I have various comments on MCJITStreamer itself, but I will
>> hold them
>> until we have an individual patch for that class.
>>
>>  - Daniel
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc




More information about the llvm-dev mailing list