[PATCH] D60095: [LLD][COFF] Early load PDB type server files

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: COFF/DebugTypes.cpp:33-36
+  explicit TypeServerSource(MemoryBufferRef M)
+      : TpiSource(PDB, nullptr), MB(M) {
+    loadPDB();
+  }
----------------
ruiu wrote:
> If a constructor can fail, one pattern to handle that is to make the constructor private and instead provide `static ErrorOr<TypeServerSource> getInstance()` function. In that function, you can load a PDB and then instantiate TypeServerSouce only when the file loading succeeded. If failed, you can return an error.
As suggested, added `static Expected<TypeServerSource *> TypeServerSource::getInstance()` and replaced previous `lld::coff::makeTypeServerSource()` by `lld::coff::loadTypeServerSource()` to be consistent.

Note - PDB load errors are simply queued in `TypeServerSource::Instance`, and displayed later by `TypeServerSource::findFromFile()` which is called by `PDBLinker::maybeMergeTypeServerPDB()`.
I've tried displaying the errors on the spot, but at that point we don't know who requested loading of that PDB, and it feels wrong to decorelate the PDB errors from the requesting OBJ:
```
lld-link: warning: 'bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing
...
... (various other warnings)
...
lld-link: warning: Cannot use debug info for 'F:\svn\build\tools\lld\test\COFF\Output\pdb-type-server-native-errors.yaml.tmp.obj' [LNK4099]
>>> failed to load reference 'bad-block-size.pdb': (see above)
```

Given it's not a fatal error, I prefer the current behavior, it feels better from a user perspective:
```
ld-link: warning: Cannot use debug info for 'F:\svn\build\tools\lld\test\COFF\Output\pdb-type-server-native-errors.yaml.tmp.obj' [LNK4099]
>>> failed to load reference 'bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing
```


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60095/new/

https://reviews.llvm.org/D60095