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

[Aaron Ballman via llvm-commits]

On Tue, Jun 14, 2016 at 2:13 PM, Zachary Turner via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> 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.

I agree, I don't think asserts are the right way to go here. We should
be resilient against unsanitized inputs.

~Aaron