[PATCH] D26253: [CodeView] Hook CodeViewRecordIO up to the type serialization path.

Adrian McCarthy via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 2 17:00:59 PDT 2016


amccarth added inline comments.


================
Comment at: include/llvm/DebugInfo/CodeView/TypeRecordMapping.h:22
+class StreamWriter;
+}
 namespace codeview {
----------------
zturner wrote:
> amccarth wrote:
> > These forward declarations seem unnecessary.  The TypeRecordMapping constructors are defined in this header, so I believe you need the complete definitions of msf::StreamReader and msf::StreamWriter, and you have the complete definitions, because CodeViewRecordIO.h includes them transitively (because it needs the complete types).
> The constructors are defined, but they only accept references.  So a full definition is not needed for these constructors.  While it's true that `CodeViewRecordIO.h` includes them transitively, we should not have one header be dependent on implementation details of another header.  If the code in `CodeViewRecordIO.h` moved into a CPP file, then this code could subsequently break.
Yes, these constructors take a reference, but the member initializers take the parameters by value, so I'm pretty sure you need complete definitions here.


https://reviews.llvm.org/D26253





More information about the llvm-commits mailing list