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

[Bob Haarman via llvm-commits]

inglorion added a comment.

Overall looks good. I have a couple of questions I have posted inline. Once those are resolved, I'll accept.



================
Comment at: include/llvm/DebugInfo/CodeView/CodeViewRecordIO.h:26
 
+#include <stdint.h>
+
----------------
Out of curiosity, why move this include? And, while no the subject, why not use cstdint?


================
Comment at: lib/DebugInfo/CodeView/CodeViewRecordIO.cpp:148
 Error CodeViewRecordIO::mapGuid(StringRef &Guid) {
+  if (16 > maxFieldLength())
+    return make_error<CodeViewError>(cv_error_code::insufficient_buffer);
----------------
Do we need to perform this check when reading, too? If not, could you move it after the assert(Guid.size() ...)?

Also, I would make a symbolic constant, particularly since it's repeated a number of times. Believe it or not, it wasn't immediately clear to me where the 16 came from (yes, I'm having my caffeine now).


================
Comment at: lib/DebugInfo/CodeView/TypeRecordMapping.cpp:119
+  // followed by the subrecord, followed by a continuation, and that entire
+  // sequence spawns `MaxRecordLength` bytes.  So the record's length is
+  // calculated as follows.
----------------
s/spawns/spans/ ?


https://reviews.llvm.org/D26253