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

[Martell Malone via Phabricator via llvm-commits]

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


================
Comment at: lib/Object/COFFImportFile.cpp:167
+  // Create a weak external file which is described in PE/COFF Aux Format 3.
+  NewArchiveMember createWeakExternal(StringRef Sym, StringRef Weak, bool imp);
 };
----------------
ruiu wrote:
> LLVM coding style: imp -> Imp
> 
> (As I said, I really appreciate if you carefully review your patch yourself before sending it so that I don't need to point out all these style errors. It seems we are spending too much time on these kind of stuff and am honestly a bit tired of pointing them out.)
Damn it, I really thought we had everything that time.
I don't know how we missed that in the previous iterations.
I assume it just became more apparent when `std::vector<uint8_t> &Buffer` was removed.
I just read through everything extra carefully and I ran clang-format again.
Updating now.


================
Comment at: lib/Object/COFFImportFile.cpp:484-486
+  static std::vector<uint8_t> Buffer;
+  static const uint32_t NumberOfSections = 1;
+  static const uint32_t NumberOfSymbols = 5;
----------------
ruiu wrote:
> Why static?
The reason for doing this was to not have to initialise the variables on every call to the function.

createNullImportDescriptor has
`static const uint32_t NumberOfSections = 1;`
`static const uint32_t NumberOfSymbols = 1;`

createImportDescriptor has
`static const uint32_t NumberOfSections = 2;`
`static const uint32_t NumberOfSymbols = 7;`
`static const uint32_t NumberOfRelocations = 3;`

createNullThunk has
`static const uint32_t NumberOfSections = 2;`
`static const uint32_t NumberOfSymbols = 1;`

We do not need the variables for the lifetime of the program but if I am to remove `static` here it should also be done for the other functions.

Which option will I go with?


Repository:
  rL LLVM

https://reviews.llvm.org/D29892