[PATCH] D33475: [pdb] pad source file name buffer at the end instead of the beginning

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: test/DebugInfo/PDB/Inputs/source-names-2.yaml:1-8
+---
+DbiStream:
+  Modules:
+    - Module:          'C:\src\test.obj'
+      ObjFile:         'C:\src\test.obj'
+      SourceFiles:
+        - 'C:\src\test.cc'
----------------
Instead of having two different yaml files that are largely similar, how about 1 file with multiple source files?

```
---
DbiStream:
  Modules:
    - Module:          'C:\src\test.obj'
      ObjFile:         'C:\src\test.obj'
      SourceFiles:
        - 'C:\src\test.c'
        - 'C:\src\testx.c'
        - 'C:\src\testxx.c'
        - 'C:\src\testxxx.c'
...
```


================
Comment at: test/DebugInfo/PDB/pdbdump-align-source-names.test:1-7
+# Module source file names are contained in the file info substream. The
+# substream is padded to a multiple of 4 bytes, and this padding must come
+# after the file names. Putting it before the file names results in the
+# possibility of source file names being read as empty or truncated. This test
+# uses file names of two different lengths to make sure at least one hits the
+# case where padding is needed and verifies that file names are correct in
+# both cases.
----------------
It feels a bit like exposing too much of an implementation detail to say that we're testing padding of an internal field.  Just call it a source file test (since we apparently didn't have one at all before)


================
Comment at: test/DebugInfo/PDB/pdbdump-align-source-names.test:16-20
+CHECK1: SourceFiles:
+CHECK1: 'C:\src\test.c'
+
+CHECK2: SourceFiles:
+CHECK2: 'C:\src\test.cc'
----------------
With one input file, this can just be:

```
CHECK: SourceFiles:
CHECK-NEXT: 'C:\src\test.c`
CHECK-NEXT: 'C:\src\testx.c`
CHECK-NEXT: 'C:\src\testxx.c`
CHECK-NEXT: 'C:\src\testxxx.c`
```


https://reviews.llvm.org/D33475