[Lldb-commits] [PATCH] D18381: Decouple ClangASTContext from DWARF

[Greg Clayton via lldb-commits]

clayborg added a comment.

I

In http://reviews.llvm.org/D18381#381457, @zturner wrote:

> I guess my point was just that it seems a little weird to treat DWARF specially.  It's just another debug info format (albeit one that most compilers support), so by baking into the supposedly generic interface the requirement that "every type system must be able to support DWARF" seems a little strange.


We can change the TypeSystem version of this function to not be pure virtual, and have a default implementation that returns nullptr. I do think it is valid to have a function that says "if you want to support parsing DWARF with this type system, please return a DWARFASTParser subclass that can convert DWARF into your types". I see no problem with this.

> I agree it solves the problem and everything will work, it just seems more correct from a design standpoint to have an enum for debug info type and expose one method called `GetDebugInfoParser` that takes the enum.


I don't agree. DWARFASTParser has very specific DWARF parsing functions on it that make no sense to anyone else. Making a enum doesn't help with this, nor does making a common base class that can't do anything for anyone. So I firmly believe this is valid. Mostly because DWARF supports so many languages and each language might have its own type system. A single DWARF file could have Java, C, C++, and Go all in the same DWARF file. Each type system can provide its own DWARFASTParser. Take a look at the DWARFASTParser, most of it is very DWARF specific.


http://reviews.llvm.org/D18381