[PATCH] D116434: [lld-link] Replace LazyObjFile with lazy ObjFile/BitcodeFile

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/COFF/SymbolTable.cpp:40
   log("Reading " + toString(file));
-  file->parse();
+  if (file->lazy) {
+    if (auto *f = dyn_cast<BitcodeFile>(file))
----------------
aganea wrote:
> I wonder if it's not better to leave types to do their own testing of `lazy` internally, in `Type::parse()`, and act accordingly (instead of doing it outside like here).
> Same thing applies to pushing to the instances, shouldn't `parse()` do it internally? (but that could be done later)
> We should be ending with just `file->parse();` here.
The reason `parse` calling `parseLazy` may feel better is because the function is in `SymbolTable` (while it can be placed into `InputFile`, too), so `lazy` has to be public to be accessed here.

If the function is in `InputFile`, this function dispatching to `parse` and `parseLazy` will be superior in my view as we can avoid the slightly unnecessary abstraction that `parse` returns `parseLazy`...

---

If PE/COFF needs the `lazyBitcodeFiles` as ELF does (Bazel distributed ThinLTO requirement), handling `lazy` in this function may be better, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116434