[PATCH] D45213: [COFF][LLD] Add link support for precompiled headers .objs

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: lld/trunk/COFF/PDB.cpp:425-426
+    // with the precompiled headers object type stream.
+    Types.setUnderlyingStream(
+        Types.getUnderlyingStream().drop_front(FirstType->length()));
+  }
----------------
zturner wrote:
> I almost wonder if we should add `CVTypeArray::drop_front()`.  You don't have to do it in this patch, just something I thought of.
I went ahead and added `VarStreamArray::drop_front()`, please let me know if that sounds good.


================
Comment at: llvm/trunk/lib/DebugInfo/CodeView/TypeStreamMerger.cpp:100
+                         ArrayRef<GloballyHashedType> Hashes,
+                         Optional<EndPrecompRecord> &EP);
 
----------------
rnk wrote:
> I was hoping to come up with some way to avoid the extra out parameter, but that's my only real concern. Structurally, everything else looks like type servers, and it's pretty straightforward.
I agree - in a subsequent chage we could fold the "merge" behavior into a single function, add a initializer struct, and call the function with a initializer list instead, ie. `mergeRecords({Dest, Types, Hashes, EP})`. That way we could accomodate different merging scenarios, and have only one codepath.


https://reviews.llvm.org/D45213