[PATCH] D59053: [LLD][COFF] Early dependency detection

[Alexandre Ganea via Phabricator via llvm-commits]

aganea marked an inline comment as done.
aganea added inline comments.


================
Comment at: COFF/InputFiles.h:164-178
+  enum TypeStreamKind { Regular, PCH, UsingPCH, UsingPDB };
+
+  // If the OBJ has a .debug$T stream, this tells which kind is it.
+  llvm::Optional<TypeStreamKind> DebugTypesKind;
+  
+  // The .debug$T stream if there's one.
+  llvm::Optional<llvm::codeview::CVTypeArray> DebugTypes;
----------------
rnk wrote:
> I'm starting to feel like mergeDebugT should really be behind a virtual interface, something like this:
> ```
> class TPISource { // TpiSource?
> public:
>   virtual CVIndexMap *mergeTPI(ObjFile *File, CVIndexMap *ObjectIndexMap) = 0;
> };
> ...
> class SectionTPISource { ... }; // ObjectTPISource? RegularTPISource? Z7TPISource? TpiSource?
> class TypeServerTPISource { ... };
> class PrecompTPISource { ... };
> class UsePrecompTPISource { ... };
> ```
> 
> The class names I came up with are kind of crummy, though.
> 
> I feel like 4 public, optional members of ObjFile leaks a lot of PDB details into the rest of the linker. Instead, we could have a pointer to some interface. Do you think this, or something like it, is worth doing now, ever, or later?
> 
> The "regular" or .debug$T based type merger could just be some stateless global variable that all the regular objs point to, since we pass the object file in through the interface, and that's all the info it needs. Every file using a type server would point to the same TypeServerTPISource. I'm not sure how the PCH stuff would work, since I'm less familiar.
Yes you're completly right - somehow I started doing that at first, but there were too many changes, so I meant to do this incrementally. I'll lay out this new class hierarchy in this patch, and then I'll implement `mergeTPI()` in a subsequent patch.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D59053