Have writeCOFFWriter return Expected<unique_ptr>.

[ecbeckmann]

Summary

Have writeCOFFWriter return Expected<unique_ptr> instead of requiring being passed an uninitialized unique_ptr.

[ruiu]

llvm/lib/Object/WindowsResource.cpp

312–313

Do you have to explicitly delete these copy ctors?

[ecbeckmann]

Removed deleted constructors.

[ecbeckmann]

llvm/lib/Object/WindowsResource.cpp

312–313

oh sorry, i meant to remove that

[ruiu]

LGTM

lld/COFF/DriverUtils.cpp

for (MemoryBufferRef MB : MBs) {
     std::unique_ptr<object::Binary> Bin = check(object::createBinary(MB));
     object::WindowsResource *RF = dyn_cast<object::WindowsResource>(Bin.get());
     if (!RF)
       fatal("cannot compile non-resource file as resource");
     if (auto EC = Parser.parse(RF))
       fatal(EC, "failed to parse .res file");
   }
 
-  std::unique_ptr<MemoryBuffer> MB;
-  if (auto EC =
-          llvm::object::writeWindowsResourceCOFF(MB, Config->Machine, Parser))
-    fatal(EC, "failed to write resources to buffer");
-
-  return MB;
+  Expected<std::unique_ptr<MemoryBuffer>> E =
+      llvm::object::writeWindowsResourceCOFF(Config->Machine, Parser);
+  if (!E)
+    fatal(errorToErrorCode(E.takeError()), "failed to write .res to COFF");
+  return std::move(E.get());
 }
 
 // Run MSVC link.exe for given in-memory object files.
 // Command line options are copied from those given to LLD.
 // This is for the /msvclto option.
 void runMSVCLinker(std::string Rsp, ArrayRef<StringRef> Objects) {
   // Write the in-memory object files to disk.
   std::vector<TemporaryFile> Temps;

llvm/include/llvm/Object/WindowsResource.h

public:
   };
 
 private:
   TreeNode Root;
   std::vector<std::vector<uint8_t>> Data;
   std::vector<std::vector<UTF16>> StringTable;
 };
 
-Error writeWindowsResourceCOFF(std::unique_ptr<MemoryBuffer> &OutputBuffer,
-                               llvm::COFF::MachineTypes MachineType,
-                               const WindowsResourceParser &Parser);
+Expected<std::unique_ptr<MemoryBuffer>>
+writeWindowsResourceCOFF(llvm::COFF::MachineTypes MachineType,
 
 } // namespace object
 } // namespace llvm
 
 #endif

llvm/lib/Object/WindowsResource.cpp

uint32_t WindowsResourceParser::TreeNode::getTreeSize() const {
   for (auto const &Child : IDChildren) {
     Size += Child.second->getTreeSize();
   }
   return Size;
 }
 
 class WindowsResourceCOFFWriter {
 public:
-  WindowsResourceCOFFWriter(std::unique_ptr<MemoryBuffer> &OutputBuffer,
-                            COFF::MachineTypes MachineType,
+  WindowsResourceCOFFWriter(COFF::MachineTypes MachineType,
                             const WindowsResourceParser &Parser, Error &E);
-  Error write();
+  std::unique_ptr<MemoryBuffer> write();
 
 private:
   void performFileLayout();
   void performSectionOneLayout();
   void performSectionTwoLayout();
   void writeCOFFHeader();
   void writeFirstSectionHeader();
   void writeSecondSectionHeader();
   void writeFirstSection();
   void writeSecondSection();
   void writeSymbolTable();
   void writeStringTable();
   void writeDirectoryTree();
   void writeDirectoryStringTable();
   void writeFirstSectionRelocations();
-  std::unique_ptr<MemoryBuffer> &OutputBuffer;
+  std::unique_ptr<MemoryBuffer> OutputBuffer;
   char *BufferStart;
   uint64_t CurrentOffset = 0;
   COFF::MachineTypes MachineType;
   const WindowsResourceParser::TreeNode &Resources;
   const ArrayRef<std::vector<uint8_t>> Data;
   uint64_t FileSize;
   uint32_t SymbolTableOffset;
   uint32_t SectionOneSize;
   uint32_t SectionOneOffset;
   uint32_t SectionOneRelocations;
   uint32_t SectionTwoSize;
   uint32_t SectionTwoOffset;
   const ArrayRef<std::vector<UTF16>> StringTable;
   std::vector<uint32_t> StringTableOffsets;
   std::vector<uint32_t> DataOffsets;
   std::vector<uint32_t> RelocationAddresses;
 };
 
 WindowsResourceCOFFWriter::WindowsResourceCOFFWriter(
-    std::unique_ptr<MemoryBuffer> &OutputBuffer, COFF::MachineTypes MachineType,
-    const WindowsResourceParser &Parser, Error &E)
-    : OutputBuffer(OutputBuffer), MachineType(MachineType),
-      Resources(Parser.getTree()), Data(Parser.getData()),
-      StringTable(Parser.getStringTable()) {
+    COFF::MachineTypes MachineType, const WindowsResourceParser &Parser,
+    Error &E)
+    : MachineType(MachineType), Resources(Parser.getTree()),
+      Data(Parser.getData()), StringTable(Parser.getStringTable()) {
   performFileLayout();
 
   OutputBuffer = MemoryBuffer::getNewMemBuffer(FileSize);
 }
 
 void WindowsResourceCOFFWriter::performFileLayout() {
   // Add size of COFF header.
   FileSize = COFF::Header16Size;

 
 static std::time_t getTime() {
   std::time_t Now = time(nullptr);
   if (Now < 0 || !isUInt<32>(Now))
     return UINT32_MAX;
   return Now;
 }
 
-Error WindowsResourceCOFFWriter::write() {
+std::unique_ptr<MemoryBuffer> WindowsResourceCOFFWriter::write() {
   BufferStart = const_cast<char *>(OutputBuffer->getBufferStart());
 
   writeCOFFHeader();
   writeFirstSectionHeader();
   writeSecondSectionHeader();
   writeFirstSection();
   writeSecondSection();
   writeSymbolTable();
   writeStringTable();
 
-  return Error::success();
+  return std::move(OutputBuffer);
 }
 
 void WindowsResourceCOFFWriter::writeCOFFHeader() {
   // Write the COFF header.
   auto *Header = reinterpret_cast<coff_file_header *>(BufferStart);
   switch (MachineType) {
   case COFF::IMAGE_FILE_MACHINE_ARMNT:
     Header->Machine = COFF::IMAGE_FILE_MACHINE_ARMNT;
case COFF::IMAGE_FILE_MACHINE_I386:
       break;
     default:
       Reloc->Type = 0;
     }
     CurrentOffset += sizeof(coff_relocation);
   }
 }
 
-Error writeWindowsResourceCOFF(std::unique_ptr<MemoryBuffer> &OutputBuffer,
-                               COFF::MachineTypes MachineType,
-                               const WindowsResourceParser &Parser) {
+Expected<std::unique_ptr<MemoryBuffer>>
+writeWindowsResourceCOFF(COFF::MachineTypes MachineType,
   Error E = Error::success();
-  WindowsResourceCOFFWriter Writer(OutputBuffer, MachineType, Parser, E);
+  WindowsResourceCOFFWriter Writer(MachineType, Parser, E);
   if (E)
-    return E;
+    return std::move(E);
   return Writer.write();
 }
 
 } // namespace object
 } // namespace llvm

llvm/tools/llvm-cvtres/llvm-cvtres.cpp

 
 void error(Error EC) {
   if (!EC)
     return;
   handleAllErrors(std::move(EC),
                   [&](const ErrorInfoBase &EI) { reportError(EI.message()); });
 }
 
+template <typename T> T error(Expected<T> EC) {
+  if (!EC)
+    error(EC.takeError());
+  return std::move(EC.get());
+}
+
 int main(int argc_, const char *argv_[]) {
   sys::PrintStackTraceOnErrorSignal(argv_[0]);
   PrettyStackTraceProgram X(argc_, argv_);
 
   ExitOnErr.setBanner("llvm-cvtres: ");
 
   SmallVector<const char *, 256> argv;
   SpecificBumpPtrAllocator<char> ArgAllocator;
for (const auto &File : InputFiles) {
     Binary &Binary = *BinaryOrErr.get().getBinary();
 
     WindowsResource *RF = dyn_cast<WindowsResource>(&Binary);
     if (!RF)
       reportError(File + ": unrecognized file format.\n");
 
     if (Verbose) {
       int EntryNumber = 0;
-      Expected<ResourceEntryRef> EntryOrErr = RF->getHeadEntry();
-      if (!EntryOrErr)
-        error(EntryOrErr.takeError());
-      ResourceEntryRef Entry = EntryOrErr.get();
+      ResourceEntryRef Entry = error(RF->getHeadEntry());
       bool End = false;
       while (!End) {
         error(Entry.moveNext(End));
         EntryNumber++;
       }
       outs() << "Number of resources: " << EntryNumber << "\n";
     }
 
     error(Parser.parse(RF));
   }
 
   if (Verbose) {
     Parser.printTree(outs());
   }
 
-  std::unique_ptr<MemoryBuffer> OutputBuffer;
-  error(llvm::object::writeWindowsResourceCOFF(OutputBuffer, MachineType,
-                                               Parser));
+  std::unique_ptr<MemoryBuffer> OutputBuffer =
+      error(llvm::object::writeWindowsResourceCOFF(MachineType, Parser));
   auto FileOrErr =
       FileOutputBuffer::create(OutputFile, OutputBuffer->getBufferSize());
   if (!FileOrErr)
     reportError(OutputFile, FileOrErr.getError());
   std::unique_ptr<FileOutputBuffer> FileBuffer = std::move(*FileOrErr);
   std::copy(OutputBuffer->getBufferStart(), OutputBuffer->getBufferEnd(),
             FileBuffer->getBufferStart());
   error(FileBuffer->commit());