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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:72
+MemoryBufferRef openFile(StringRef Path) {
+  llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> MB =
+      MemoryBuffer::getFile(Path);
----------------
Remove `llvm::`.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:99-100
+int llvm::dlltoolDriverMain(llvm::ArrayRef<const char *> ArgsArr) {
+  BumpPtrAllocator Alloc;
+  StringSaver Saver(Alloc);
+  COFF::MachineTypes Machine = IMAGE_FILE_MACHINE_UNKNOWN;
----------------
These variables are dead. Please remove. (Please take a look at your code more carefully so that you won't add dead code.)


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:101
+  StringSaver Saver(Alloc);
+  COFF::MachineTypes Machine = IMAGE_FILE_MACHINE_UNKNOWN;
+  DllOptTable Table;
----------------
Move this to Line 134 so that the variable definition is closer to the use of the variable.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:103
+  DllOptTable Table;
+  std::string Path;
+  MemoryBufferRef MB;
----------------
Move this to line 134.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:104
+  std::string Path;
+  MemoryBufferRef MB;
+  unsigned MissingIndex;
----------------
Move this to line 131.


================
Comment at: lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp:158
+
+  if (!Path.length())
+    Path = getImplibPath(Def->OutputFile);
----------------
Use `empty()` instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D29892