[PATCH] D21220: [pdb] Actually write a PDB to the disk

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: include/llvm/DebugInfo/CodeView/StreamRef.h:96
@@ -93,1 +95,3 @@
 
+  StreamRef &operator=(const StreamRef &Other) {
+    Stream = Other.Stream;
----------------
ruiu wrote:
> Is this different from the default copy assignment operator?
I don't think so, this might have slipped in accidentally.  

================
Comment at: include/llvm/DebugInfo/CodeView/StreamWriter.h:44
@@ -43,2 +43,3 @@
 
-  template <typename T> Error writeObject(const T &Obj) {
+  template <typename T, typename = std::enable_if_t<!std::is_pointer<T>::value>>
+  Error writeObject(const T &Obj) {
----------------
ruiu wrote:
> Can you use static_assert instead of enable_if_t?
Probably so, I will try it.

================
Comment at: lib/DebugInfo/PDB/Raw/PDBFile.cpp:287
@@ +286,3 @@
+
+Error PDBFile::setSuperBlock(const SuperBlock *Block) {
+  SB = Block;
----------------
ruiu wrote:
> We shouldn't meet the error conditions that we are checking in this function unless there's a bug in PDB producer code or YAML reader code (YAML reader should do reasonable semantic checking), so they should be asserts.
Technically a PDB file is a program input, we could generate one through fuzzing for example.  Also we can't assume we are the only producer of PDB files.  And since this will be in library code, I don't know if I feel comfortable asserting on conditions of the program input.


http://reviews.llvm.org/D21220