[PATCH] D41922: [WebAssembly] When loading libraries look for companion `.imports` file

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: wasm/Driver.cpp:176-185
+  if (identify_magic(MBRef.getBuffer()) == file_magic::archive) {
     Files.push_back(make<ArchiveFile>(MBRef));
-  else
+    SmallString<128> ImportFile = Path;
+    path::replace_extension(ImportFile, ".imports");
+    if (fs::exists(ImportFile))
+      processImportFile(ImportFile.str());
+  } else {
----------------
ruiu wrote:
> This looks suspicious --
> 
> 1. It loads an entire .import file for an .a file, even if we won't end up linking any object file from that archive file. That doesn't make much sense to me. Shouldn't this be per-file?
> 
> 2. IMO a side-by-side magic text file is in general not a good idea. We don't have such notion in Unix linker, and we get used to copy .a and .o files to other directories expecting that they would just work fine. This seems error-prone.
I agree its not ideal.  But it is incrementally better than that current solution which requires the user (normally the clang driver) to pass extra linker args.   The long term solution probably involved marking the symbold with __attribute__(something) in the source and marking those symbols in the object format.    So I'd like to land this incremental solution now.  We can leave the bug open to create a more robust solution.

This change will also allow us to remove the -allow-undefined-file option, which will be nice cleanup.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41922