[PATCH] D29892: ar: add llvm-dlltool support

[Martell Malone via Phabricator via llvm-commits]

martell marked 3 inline comments as done.
martell added inline comments.


================
Comment at: lib/Object/COFFImportFile.cpp:549-550
+        sizeof(uint32_t) + Sym.size() + 1 + 6;
+    writeStringTable(Buffer, {std::string("__imp_").append(Sym),
+                              std::string("__imp_").append(Weak)});
+  } else {
----------------
ruiu wrote:
> Are these the same as
> 
>   "__imp_" + Sym
> 
> and
> 
>   "__imp_" + Weak
> 
> ? If so, write that way.
`writeStringTable(Buffer, {"__imp_" + Sym, "__imp_" + Weak});`
error: no matching function for call to 'writeStringTable'
note: candidate function not viable: cannot convert initializer list argument to 'ArrayRef<const std::string>' (aka 'ArrayRef<const basic_string<char, char_traits<char>, allocator<char> > >')

Using Twine fails here also and converting is pointless as we might as well just use std::string at that point.

E.Name and ENameExt use this same format `E.Name = (std::string("_").append(E.Name));`
Seems appropriate considering the type.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:144-146
+  // Do this after the parser because parseCOFFModuleDefinition sets OutputFile.
+  if (auto *Arg = Args.getLastArg(OPT_D))
+    Def->OutputFile = Arg->getValue();
----------------
ruiu wrote:
> Is `OutputFile` guaranteed to have some non-empty value at this point?
`parseOne` in `parseCOFFModuleDefinition` sets it if `Name` is in the definition.
Is possible if -D is not passed and no name is in the def file.
I was letting `writeArchive` error out on this but we can exit sooner here.




Repository:
  rL LLVM

https://reviews.llvm.org/D29892