[PATCH] D42514: [ThinLTO/gold] Write empty imports even for modules with symbols

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/tools/gold/gold-plugin.cpp:873
+      createLTO([&ObjectFilenames](const std::string &Identifier) {
+        ObjectFilenames.erase(Identifier);
+      });
----------------
Thinking through this some more, this seems a little dicey, because it assumes that createLTO will never try to access these filename strings that are owned by this set after the callback. In fact, that doesn't even seem to be the case currently (ModulePath which is a StringRef of the module identifier is passed to EmitImportsFiles after this callback is invoked). Rather than try to fix that, since it would still be fragile, I suggest making this a StringMap and noting in the map whether the file was written.


================
Comment at: llvm/tools/llvm-lto2/llvm-lto2.cpp:245
   ThinBackend Backend;
-  if (ThinLTODistributedIndexes)
-    Backend = createWriteIndexesThinBackend("", "", true, "");
-  else
+  if (ThinLTODistributedIndexes) {
+    Backend =
----------------
tejohnson wrote:
> I think the only change here is the addition of the braces - I assume that was from some debugging code that was since removed, so the braces and the change to this file can be removed.
Oh I see, there was a change here. In any case, please remove the added braces which aren't necessary.


https://reviews.llvm.org/D42514