<div dir="ltr"><br><div class="gmail_quote">On Thu Jan 29 2015 at 5:59:54 PM Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> On Jan 29, 2015, at 2:36 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:<br>
><br>
> Hi aprantl, echristo, dblaikie, friss,<br>
><br>
> 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.<br>
><br>
> This is purely a mechanical / build system change.<br>
><br>
> I successfully built LLVM using CMake and ninja check-llvm returned no failures.<br>
><br>
> I'm not able to test the make/autoconf build, but I did update the relevant Makefiles, so I believe it should work.<br>
<br>
Why aren’t you able to do a configure/make build?<br></blockquote><div>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 :)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> [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 :)]<br>
<br>
Don’t worry about that, this should not slow you down. Moving the files is easy enough.<br>
<br>
> <a href="http://reviews.llvm.org/D7269" target="_blank">http://reviews.llvm.org/D7269</a><br>
><br>
> Files:<br>
>  include/llvm/DebugInfo/<u></u>DIContext.h<br>
>  include/llvm/DebugInfo/DWARF/<u></u>DIContext.h<br>
>  include/llvm/DebugInfo/DWARF/<u></u>DWARFAbbreviationDeclaration.h<br>
<br>
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.<br></blockquote>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.  <br><div><br></div><div>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.  :(</div></div></div>