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

Bob Haarman via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 2 14:16:43 PDT 2016

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/ ?


More information about the llvm-commits mailing list