[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