[LLVMdev] Win32 COFF Support

[Daniel Dunbar]

Hi Nathan,

I'm not quite ready to apply this patch, but I now have time again to
work on getting it in.

The main reason I am not willing to put it in yet is (a) the size, and
(b) the inconsistences with LLVM style. Please fix the following
things:
  - Watch out for case mismatches, i.e., #include "WinCoff.h" when the
header is actually WinCOFF.h.
  - The patch doesn't have a consistent newline convention, it is a
mix of DOS and Unix.
  - if( -> if (
  - A [1] -> A[1]
  - Braces go on the same line as if/else.
  - Please use AddFragment instead of add_fragment, etc, and we tend
to only use a leading lower case for accessor methods (getFoo) or
trivial inlines functions, but not for methods which do work or have
substantial side effects.
  - Please use complete sentences in comments, i.e. capitalized and
ending with proper punctuation.
  - Please don't use an extra newline following 'for' loops,
conditionals, or type declarations.
  - Please see http://llvm.cs.uiuc.edu/docs/CodingStandards.html if
you haven't already.

There are also some meatier things I would like changed:
  - Please split the patch into at least four parts:
    (1) Add WinCOFFStreamer, with a stub implementation.
    (2) Add WinCOFFObjectWriter, with a stub implementation.
    (3) Fill in WinCOFFStreamer.
    (4) Fill in WinCOFFObjectWriter.
   The first two parts should be trivial, and as small as possible. It
is useful to get them in first to make subsequent patches easier to
read.
  - Please put the COFF constants which are in WinCOFF.h in
include/llvm/Support/COFF.h.
    o This should only be the stuff which pertains to the file format,
and is not implementation dependent (i.e., no structure definitions
unless they exactly match the file format, and are pure C structs).
  - Please move the remaining content in WinCOFF.h into
WinCOFFObjectWriter.cpp, since it is implementation dependent and
shouldn't need to be exposed.
  - I have some more comments about the actual implementation, but I'd
like to see smaller patches before getting in to details.

 - Daniel

On Tue, Jun 15, 2010 at 12:47 AM, Nathan Jeffords
<blunted2night at gmail.com> wrote:
> I have updated my patch based on Chris'es feedback. I removed the
> dbgout_calls macro, but left others in place for now. If there are no
> objections, I would like to commit this tomorrow evening (~7PM GMT-7). I
> have compiled and tested it on MSVC, with Michaels testing code and it looks
> good. Once this is committed, Michaels patch can be applied.
> -Nathan
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
>