[llvm-commits] [lld] Supporting archive libraries in lld

[Shankar Easwaran]

Hi Nick,

Thanks for your comments.

+    if ((ec = _options.reader()->parseFile(std::unique_ptr<MemoryBuffer>
+                                           (ci->getBuffer()), result)))
+      return nullptr;
+

> should be an assert() here that result.size() == 1, just to make sure parseFile() does not return multiple Files and we only use one of them.
Will add the assert.

> +{
> +  // Returns a vector of Files that are contained in the archive file
> +  error_code ReaderArchive::parseFile(StringRef path,
> +  		std::vector<std::unique_ptr<File>> &result) {
> +    error_code ec;
> +    OwningPtr<llvm::MemoryBuffer> opmb;
> +    if ((ec = llvm::MemoryBuffer::getFile(path, opmb)))
> +    	return ec;
> +
> +    std::unique_ptr<MemoryBuffer> mb(opmb.take());
> +
> +    return parseFile(std::move(mb), result);
> +  }
> ReaderArchive::parseFile() should not be needed.  The base class Reader defines readFile() which already does the above.
Will remove the function.

>>   
>>   #include <map>
>> @@ -751,20 +752,36 @@
>>   
>>       }
> The getElfArchType() is currently always called.  It only needs to be called if the filetype is ELF_Relocatable_FileType.
>
Ok.

+  std::unique_ptr<ReaderArchive> _readerArchive;
  };

> Since ReaderELF constructor always  takes a ReaderOptionsArchive parameter, you can make _readerArchive be an instance of ReaderArchive rather than an owned pointer to a ReaderArchive.
>
Will do.

Thanks

Shankar Easwaran

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation