[PATCH] Move DebugInfo to DebugInfo/DWARF

[Zachary Turner]

On Thu Jan 29 2015 at 5:59:54 PM Frédéric Riss <friss at apple.com> wrote:

>
> > On Jan 29, 2015, at 2:36 PM, Zachary Turner <zturner at google.com> wrote:
> >
> > Hi aprantl, echristo, dblaikie, friss,
> >
> > In preparation for adding PDB support to LLVM, this moves the DWARF
> parsing code to its own subdirectory under DebugInfo, and renames
> LLVMDebugInfo to LLVMDebugInfoDWARF.
> >
> > This is purely a mechanical / build system change.
> >
> > I successfully built LLVM using CMake and ninja check-llvm returned no
> failures.
> >
> > I'm not able to test the make/autoconf build, but I did update the
> relevant Makefiles, so I believe it should work.
>
> Why aren’t you able to do a configure/make build?
>
Does it even work on Windows?  I was under the impression it didn't.  I
suppose I can try it, although I've never really used it before so I'm not
familiar with how to build that way.  I just assumed it wasn't a thing on
Windows :)


>
> > [BTW, I know Frederic said he's working on a change that may conflict
> with this.  I'm happy to wait until that change is in if people desire, but
> this was pretty mechanical and quick to whip up, so figured I would put it
> up in the meantime rather than wait around and do nothing :)]
>
> Don’t worry about that, this should not slow you down. Moving the files is
> easy enough.
>
> > http://reviews.llvm.org/D7269
> >
> > Files:
> >  include/llvm/DebugInfo/DIContext.h
> >  include/llvm/DebugInfo/DWARF/DIContext.h
> >  include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h
>
> I don’t know what LLVM’s policy is with respect to include hierarchy and
> libraries. The name of the files here already nicely create a namespace. If
> there’s no policy requiring the separate directory, I think I’d slightly
> prefer to leave all the includes in include/llvm/DebugInfo/ and add the PDB
> ones in the same place with a PDB prefix. It would also make the patch much
> smaller.
>
Agree it would make the patch smaller, But it seems wrong from a modularity
standpoint.  I think all include files that are #include'able from the same
directory should be part of the same library.  I think that's the natural
expectation of other people when they look in a directory and see a list of
files, is that they can include any of them and their program will work.

If anything, I would prefer to remove DWARF from the filenames and wrap the
code in a dwarf namespace, but then that would make the patch even bigger.
 :(
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150130/591d30db/attachment.html>