[clang] 93b29d3 - Revert "[Clang][Bundler] Error reporting improvements"

Sergey Dmitriev via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 25 18:00:38 PDT 2019


Author: Sergey Dmitriev
Date: 2019-10-25T17:57:55-07:00
New Revision: 93b29d3882baf7df42e4e9bc26b977b00373ef56

URL: https://github.com/llvm/llvm-project/commit/93b29d3882baf7df42e4e9bc26b977b00373ef56
DIFF: https://github.com/llvm/llvm-project/commit/93b29d3882baf7df42e4e9bc26b977b00373ef56.diff

LOG: Revert "[Clang][Bundler] Error reporting improvements"

This reverts commit dd501045cdea1c80b6788f0266d2a79f8b412eea.

Added: 
    

Modified: 
    clang/test/Driver/clang-offload-bundler.c
    clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Removed: 
    


################################################################################
diff  --git a/clang/test/Driver/clang-offload-bundler.c b/clang/test/Driver/clang-offload-bundler.c
index 35c327c56b3b..be17b092ef34 100644
--- a/clang/test/Driver/clang-offload-bundler.c
+++ b/clang/test/Driver/clang-offload-bundler.c
@@ -52,27 +52,27 @@
 // Check errors.
 //
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i -unbundle 2>&1 | FileCheck %s --check-prefix CK-ERR1
-// CK-ERR1: error: only one input file supported in unbundling mode
-// CK-ERR1: error: number of output files and targets should match in unbundling mode
+// CK-ERR1: error: only one input file supported in unbundling mode.
+// CK-ERR1: error: number of output files and targets should match in unbundling mode.
 
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR2
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR2
-// CK-ERR2: error: number of input files and targets should match in bundling mode
+// CK-ERR2: error: number of input files and targets should match in bundling mode.
 
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR3
-// CK-ERR3: error: only one output file supported in bundling mode
-// CK-ERR3: error: number of input files and targets should match in bundling mode
+// CK-ERR3: error: only one output file supported in bundling mode.
+// CK-ERR3: error: number of input files and targets should match in bundling mode.
 
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i -unbundle 2>&1 | FileCheck %s --check-prefix CK-ERR4
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1 -inputs=%t.bundle.i -unbundle 2>&1 | FileCheck %s --check-prefix CK-ERR4
-// CK-ERR4: error: number of output files and targets should match in unbundling mode
+// CK-ERR4: error: number of output files and targets should match in unbundling mode.
 
-// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck %s -DFILE=%t.tgt2.notexist --check-prefix CK-ERR5
-// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 | FileCheck %s -DFILE=%t.bundle.i.notexist --check-prefix CK-ERR5
-// CK-ERR5: error: '[[FILE]]': {{N|n}}o such file or directory
+// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2.notexist -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR5
+// RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.i,%t.tgt1,%t.tgt2 -inputs=%t.bundle.i.notexist -unbundle 2>&1 | FileCheck %s --check-prefix CK-ERR5
+// CK-ERR5: error: Can't open file {{.+}}.notexist: {{N|n}}o such file or directory
 
-// RUN: not clang-offload-bundler -type=invalid -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s -DTYPE=invalid --check-prefix CK-ERR6
-// CK-ERR6: error: '[[TYPE]]': invalid file type specified
+// RUN: not clang-offload-bundler -type=invalid -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR6
+// CK-ERR6: error: invalid file type specified.
 
 // RUN: not clang-offload-bundler 2>&1 | FileCheck %s --check-prefix CK-ERR7
 // CK-ERR7-DAG: clang-offload-bundler: for the --type option: must be specified at least once!
@@ -81,14 +81,14 @@
 // CK-ERR7-DAG: clang-offload-bundler: for the --targets option: must be specified at least once!
 
 // RUN: not clang-offload-bundler -type=i -targets=hxst-powerpcxxle-ibm-linux-gnu,openxp-pxxerpc64le-ibm-linux-gnu,xpenmp-x86_xx-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR8
-// CK-ERR8: error: invalid target 'hxst-powerpcxxle-ibm-linux-gnu', unknown offloading kind 'hxst', unknown target triple 'powerpcxxle-ibm-linux-gnu'
-// CK-ERR8: error: invalid target 'openxp-pxxerpc64le-ibm-linux-gnu', unknown offloading kind 'openxp', unknown target triple 'pxxerpc64le-ibm-linux-gnu'
-// CK-ERR8: error: invalid target 'xpenmp-x86_xx-pc-linux-gnu', unknown offloading kind 'xpenmp', unknown target triple 'x86_xx-pc-linux-gnu'
+// CK-ERR8: error: invalid target 'hxst-powerpcxxle-ibm-linux-gnu', unknown offloading kind 'hxst', unknown target triple 'powerpcxxle-ibm-linux-gnu'.
+// CK-ERR8: error: invalid target 'openxp-pxxerpc64le-ibm-linux-gnu', unknown offloading kind 'openxp', unknown target triple 'pxxerpc64le-ibm-linux-gnu'.
+// CK-ERR8: error: invalid target 'xpenmp-x86_xx-pc-linux-gnu', unknown offloading kind 'xpenmp', unknown target triple 'x86_xx-pc-linux-gnu'.
 
 // RUN: not clang-offload-bundler -type=i -targets=openmp-powerpc64le-linux,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR9A
 // RUN: not clang-offload-bundler -type=i -targets=host-%itanium_abi_triple,host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu -inputs=%t.i,%t.tgt1,%t.tgt2 -outputs=%t.bundle.i 2>&1 | FileCheck %s --check-prefix CK-ERR9B
-// CK-ERR9A: error: expecting exactly one host target but got 0
-// CK-ERR9B: error: expecting exactly one host target but got 2
+// CK-ERR9A: error: expecting exactly one host target but got 0.
+// CK-ERR9B: error: expecting exactly one host target but got 2.
 
 //
 // Check text bundle. This is a readable format, so we check for the format we expect to find.

diff  --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
index 9d7b1ba358f9..72def7756dd7 100644
--- a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -26,17 +26,15 @@
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
-#include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
+#include "llvm/Support/raw_ostream.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/StringSaver.h"
-#include "llvm/Support/WithColor.h"
-#include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
@@ -44,7 +42,6 @@
 #include <memory>
 #include <string>
 #include <system_error>
-#include <utility>
 
 using namespace llvm;
 using namespace llvm::object;
@@ -126,37 +123,36 @@ class FileHandler {
   virtual ~FileHandler() {}
 
   /// Update the file handler with information from the header of the bundled
-  /// file.
-  virtual Error ReadHeader(MemoryBuffer &Input) = 0;
+  /// file
+  virtual void ReadHeader(MemoryBuffer &Input) = 0;
 
-  /// Read the marker of the next bundled to be read in the file. The bundle
-  /// name is returned if there is one in the file, or `None` if there are no
-  /// more bundles to be read.
-  virtual Expected<Optional<StringRef>>
-  ReadBundleStart(MemoryBuffer &Input) = 0;
+  /// Read the marker of the next bundled to be read in the file. The triple of
+  /// the target associated with that bundle is returned. An empty string is
+  /// returned if there are no more bundles to be read.
+  virtual StringRef ReadBundleStart(MemoryBuffer &Input) = 0;
 
   /// Read the marker that closes the current bundle.
-  virtual Error ReadBundleEnd(MemoryBuffer &Input) = 0;
+  virtual void ReadBundleEnd(MemoryBuffer &Input) = 0;
 
   /// Read the current bundle and write the result into the stream \a OS.
-  virtual Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
+  virtual void ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
 
   /// Write the header of the bundled file to \a OS based on the information
   /// gathered from \a Inputs.
-  virtual Error WriteHeader(raw_fd_ostream &OS,
-                            ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) = 0;
+  virtual void WriteHeader(raw_fd_ostream &OS,
+                           ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) = 0;
 
   /// Write the marker that initiates a bundle for the triple \a TargetTriple to
   /// \a OS.
-  virtual Error WriteBundleStart(raw_fd_ostream &OS,
-                                 StringRef TargetTriple) = 0;
+  virtual void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
 
   /// Write the marker that closes a bundle for the triple \a TargetTriple to \a
-  /// OS.
-  virtual Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
+  /// OS. Return true if any error was found.
+
+  virtual bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) = 0;
 
   /// Write the bundle from \a Input into \a OS.
-  virtual Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
+  virtual void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) = 0;
 };
 
 /// Handler for binary files. The bundled file will have the following format
@@ -228,7 +224,7 @@ class BinaryFileHandler final : public FileHandler {
 
   ~BinaryFileHandler() final {}
 
-  Error ReadHeader(MemoryBuffer &Input) final {
+  void ReadHeader(MemoryBuffer &Input) final {
     StringRef FC = Input.getBuffer();
 
     // Initialize the current bundle with the end of the container.
@@ -237,16 +233,16 @@ class BinaryFileHandler final : public FileHandler {
     // Check if buffer is smaller than magic string.
     size_t ReadChars = sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1;
     if (ReadChars > FC.size())
-      return Error::success();
+      return;
 
     // Check if no magic was found.
     StringRef Magic(FC.data(), sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1);
     if (!Magic.equals(OFFLOAD_BUNDLER_MAGIC_STR))
-      return Error::success();
+      return;
 
     // Read number of bundles.
     if (ReadChars + 8 > FC.size())
-      return Error::success();
+      return;
 
     uint64_t NumberOfBundles = Read8byteIntegerFromBuffer(FC, ReadChars);
     ReadChars += 8;
@@ -256,35 +252,35 @@ class BinaryFileHandler final : public FileHandler {
 
       // Read offset.
       if (ReadChars + 8 > FC.size())
-        return Error::success();
+        return;
 
       uint64_t Offset = Read8byteIntegerFromBuffer(FC, ReadChars);
       ReadChars += 8;
 
       // Read size.
       if (ReadChars + 8 > FC.size())
-        return Error::success();
+        return;
 
       uint64_t Size = Read8byteIntegerFromBuffer(FC, ReadChars);
       ReadChars += 8;
 
       // Read triple size.
       if (ReadChars + 8 > FC.size())
-        return Error::success();
+        return;
 
       uint64_t TripleSize = Read8byteIntegerFromBuffer(FC, ReadChars);
       ReadChars += 8;
 
       // Read triple.
       if (ReadChars + TripleSize > FC.size())
-        return Error::success();
+        return;
 
       StringRef Triple(&FC.data()[ReadChars], TripleSize);
       ReadChars += TripleSize;
 
       // Check if the offset and size make sense.
       if (!Offset || Offset + Size > FC.size())
-        return Error::success();
+        return;
 
       assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&
              "Triple is duplicated??");
@@ -293,31 +289,28 @@ class BinaryFileHandler final : public FileHandler {
     // Set the iterator to where we will start to read.
     CurBundleInfo = BundlesInfo.end();
     NextBundleInfo = BundlesInfo.begin();
-    return Error::success();
   }
 
-  Expected<Optional<StringRef>> ReadBundleStart(MemoryBuffer &Input) final {
+  StringRef ReadBundleStart(MemoryBuffer &Input) final {
     if (NextBundleInfo == BundlesInfo.end())
-      return None;
+      return StringRef();
     CurBundleInfo = NextBundleInfo++;
     return CurBundleInfo->first();
   }
 
-  Error ReadBundleEnd(MemoryBuffer &Input) final {
+  void ReadBundleEnd(MemoryBuffer &Input) final {
     assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
-    return Error::success();
   }
 
-  Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
+  void ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     assert(CurBundleInfo != BundlesInfo.end() && "Invalid reader info!");
     StringRef FC = Input.getBuffer();
     OS.write(FC.data() + CurBundleInfo->second.Offset,
              CurBundleInfo->second.Size);
-    return Error::success();
   }
 
-  Error WriteHeader(raw_fd_ostream &OS,
-                    ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
+  void WriteHeader(raw_fd_ostream &OS,
+                   ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
     // Compute size of the header.
     uint64_t HeaderSize = 0;
 
@@ -336,7 +329,7 @@ class BinaryFileHandler final : public FileHandler {
 
     unsigned Idx = 0;
     for (auto &T : TargetNames) {
-      MemoryBuffer &MB = *Inputs[Idx++];
+      MemoryBuffer &MB = *Inputs[Idx++].get();
       // Bundle offset.
       Write8byteIntegerToBuffer(OS, HeaderSize);
       // Size of the bundle (adds to the next bundle's offset)
@@ -347,26 +340,25 @@ class BinaryFileHandler final : public FileHandler {
       // Triple
       OS << T;
     }
-    return Error::success();
   }
 
-  Error WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
-    return Error::success();
-  }
+  void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {}
 
-  Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
-    return Error::success();
+  bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
+    return false;
   }
 
-  Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
+  void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     OS.write(Input.getBufferStart(), Input.getBufferSize());
-    return Error::success();
   }
 };
 
 /// Handler for object files. The bundles are organized by sections with a
 /// designated name.
 ///
+/// In order to bundle we create an IR file with the content of each section and
+/// use incremental linking to produce the resulting object.
+///
 /// To unbundle, we just copy the contents of the designated section.
 class ObjectFileHandler final : public FileHandler {
 
@@ -376,19 +368,26 @@ class ObjectFileHandler final : public FileHandler {
   /// Return the input file contents.
   StringRef getInputFileContents() const { return Obj->getData(); }
 
-  /// Return bundle name (<kind>-<triple>) if the provided section is an offload
-  /// section.
-  static Expected<Optional<StringRef>> IsOffloadSection(SectionRef CurSection) {
-    Expected<StringRef> NameOrErr = CurSection.getName();
-    if (!NameOrErr)
-      return NameOrErr.takeError();
+  /// Return true if the provided section is an offload section and return the
+  /// triple by reference.
+  static bool IsOffloadSection(SectionRef CurSection,
+                               StringRef &OffloadTriple) {
+    StringRef SectionName;
+    if (Expected<StringRef> NameOrErr = CurSection.getName())
+      SectionName = *NameOrErr;
+    else
+      consumeError(NameOrErr.takeError());
+
+    if (SectionName.empty())
+      return false;
 
     // If it does not start with the reserved suffix, just skip this section.
-    if (!NameOrErr->startswith(OFFLOAD_BUNDLER_MAGIC_STR))
-      return None;
+    if (!SectionName.startswith(OFFLOAD_BUNDLER_MAGIC_STR))
+      return false;
 
     // Return the triple that is right after the reserved prefix.
-    return NameOrErr->substr(sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1);
+    OffloadTriple = SectionName.substr(sizeof(OFFLOAD_BUNDLER_MAGIC_STR) - 1);
+    return true;
   }
 
   /// Total number of inputs.
@@ -410,67 +409,71 @@ class ObjectFileHandler final : public FileHandler {
 
   ~ObjectFileHandler() final {}
 
-  Error ReadHeader(MemoryBuffer &Input) final { return Error::success(); }
+  void ReadHeader(MemoryBuffer &Input) final {}
 
-  Expected<Optional<StringRef>> ReadBundleStart(MemoryBuffer &Input) final {
+  StringRef ReadBundleStart(MemoryBuffer &Input) final {
     while (NextSection != Obj->section_end()) {
       CurrentSection = NextSection;
       ++NextSection;
 
+      StringRef OffloadTriple;
       // Check if the current section name starts with the reserved prefix. If
       // so, return the triple.
-      Expected<Optional<StringRef>> TripleOrErr =
-          IsOffloadSection(*CurrentSection);
-      if (!TripleOrErr)
-        return TripleOrErr.takeError();
-      if (*TripleOrErr)
-        return **TripleOrErr;
+      if (IsOffloadSection(*CurrentSection, OffloadTriple))
+        return OffloadTriple;
     }
-    return None;
+    return StringRef();
   }
 
-  Error ReadBundleEnd(MemoryBuffer &Input) final { return Error::success(); }
+  void ReadBundleEnd(MemoryBuffer &Input) final {}
+
+  void ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
+    // If the current section has size one, that means that the content we are
+    // interested in is the file itself. Otherwise it is the content of the
+    // section.
+    //
+    // TODO: Instead of copying the input file as is, deactivate the section
+    // that is no longer needed.
 
-  Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     Expected<StringRef> Content = CurrentSection->getContents();
-    if (!Content)
-      return Content.takeError();
+    if (!Content) {
+      consumeError(Content.takeError());
+      return;
+    }
 
     OS.write(Content->data(), Content->size());
-    return Error::success();
   }
 
-  Error WriteHeader(raw_fd_ostream &OS,
-                    ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
+  void WriteHeader(raw_fd_ostream &OS,
+                   ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
     assert(HostInputIndex != ~0u && "Host input index not defined.");
 
     // Record number of inputs.
     NumberOfInputs = Inputs.size();
-    return Error::success();
   }
 
-  Error WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
+  void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
     ++NumberOfProcessedInputs;
-    return Error::success();
   }
 
-  Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
+  bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
     assert(NumberOfProcessedInputs <= NumberOfInputs &&
            "Processing more inputs that actually exist!");
     assert(HostInputIndex != ~0u && "Host input index not defined.");
 
     // If this is not the last output, we don't have to do anything.
     if (NumberOfProcessedInputs != NumberOfInputs)
-      return Error::success();
+      return false;
 
     // Find llvm-objcopy in order to create the bundle binary.
     ErrorOr<std::string> Objcopy = sys::findProgramByName(
         "llvm-objcopy", sys::path::parent_path(BundlerExecutable));
     if (!Objcopy)
       Objcopy = sys::findProgramByName("llvm-objcopy");
-    if (!Objcopy)
-      return createStringError(Objcopy.getError(),
-                               "unable to find 'llvm-objcopy' in path");
+    if (!Objcopy) {
+      errs() << "error: unable to find 'llvm-objcopy' in path.\n";
+      return true;
+    }
 
     // We write to the output file directly. So, we close it and use the name
     // to pass down to llvm-objcopy.
@@ -490,22 +493,21 @@ class ObjectFileHandler final : public FileHandler {
     // If the user asked for the commands to be printed out, we do that instead
     // of executing it.
     if (PrintExternalCommands) {
-      errs() << "\"" << *Objcopy << "\"";
+      errs() << "\"" << Objcopy.get() << "\"";
       for (StringRef Arg : drop_begin(ObjcopyArgs, 1))
         errs() << " \"" << Arg << "\"";
       errs() << "\n";
     } else {
-      if (sys::ExecuteAndWait(*Objcopy, ObjcopyArgs))
-        return createStringError(inconvertibleErrorCode(),
-                                 "'llvm-objcopy' tool failed");
+      if (sys::ExecuteAndWait(Objcopy.get(), ObjcopyArgs)) {
+        errs() << "error: llvm-objcopy tool failed.\n";
+        return true;
+      }
     }
 
-    return Error::success();
+    return false;
   }
 
-  Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
-    return Error::success();
-  }
+  void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {}
 };
 
 /// Handler for text files. The bundled file will have the following format.
@@ -531,15 +533,15 @@ class TextFileHandler final : public FileHandler {
   size_t ReadChars = 0u;
 
 protected:
-  Error ReadHeader(MemoryBuffer &Input) final { return Error::success(); }
+  void ReadHeader(MemoryBuffer &Input) final {}
 
-  Expected<Optional<StringRef>> ReadBundleStart(MemoryBuffer &Input) final {
+  StringRef ReadBundleStart(MemoryBuffer &Input) final {
     StringRef FC = Input.getBuffer();
 
     // Find start of the bundle.
     ReadChars = FC.find(BundleStartString, ReadChars);
     if (ReadChars == FC.npos)
-      return None;
+      return StringRef();
 
     // Get position of the triple.
     size_t TripleStart = ReadChars = ReadChars + BundleStartString.size();
@@ -547,7 +549,7 @@ class TextFileHandler final : public FileHandler {
     // Get position that closes the triple.
     size_t TripleEnd = ReadChars = FC.find("\n", ReadChars);
     if (TripleEnd == FC.npos)
-      return None;
+      return StringRef();
 
     // Next time we read after the new line.
     ++ReadChars;
@@ -555,21 +557,21 @@ class TextFileHandler final : public FileHandler {
     return StringRef(&FC.data()[TripleStart], TripleEnd - TripleStart);
   }
 
-  Error ReadBundleEnd(MemoryBuffer &Input) final {
+  void ReadBundleEnd(MemoryBuffer &Input) final {
     StringRef FC = Input.getBuffer();
 
     // Read up to the next new line.
     assert(FC[ReadChars] == '\n' && "The bundle should end with a new line.");
 
     size_t TripleEnd = ReadChars = FC.find("\n", ReadChars + 1);
-    if (TripleEnd != FC.npos)
-      // Next time we read after the new line.
-      ++ReadChars;
+    if (TripleEnd == FC.npos)
+      return;
 
-    return Error::success();
+    // Next time we read after the new line.
+    ++ReadChars;
   }
 
-  Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
+  void ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     StringRef FC = Input.getBuffer();
     size_t BundleStart = ReadChars;
 
@@ -578,28 +580,22 @@ class TextFileHandler final : public FileHandler {
 
     StringRef Bundle(&FC.data()[BundleStart], BundleEnd - BundleStart);
     OS << Bundle;
-
-    return Error::success();
   }
 
-  Error WriteHeader(raw_fd_ostream &OS,
-                    ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
-    return Error::success();
-  }
+  void WriteHeader(raw_fd_ostream &OS,
+                   ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {}
 
-  Error WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
+  void WriteBundleStart(raw_fd_ostream &OS, StringRef TargetTriple) final {
     OS << BundleStartString << TargetTriple << "\n";
-    return Error::success();
   }
 
-  Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
+  bool WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
     OS << BundleEndString << TargetTriple << "\n";
-    return Error::success();
+    return false;
   }
 
-  Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
+  void WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     OS << Input.getBuffer();
-    return Error::success();
   }
 
 public:
@@ -615,60 +611,61 @@ class TextFileHandler final : public FileHandler {
 /// Return an appropriate object file handler. We use the specific object
 /// handler if we know how to deal with that format, otherwise we use a default
 /// binary file handler.
-static std::unique_ptr<FileHandler>
-CreateObjectFileHandler(MemoryBuffer &FirstInput) {
+static FileHandler *CreateObjectFileHandler(MemoryBuffer &FirstInput) {
   // Check if the input file format is one that we know how to deal with.
   Expected<std::unique_ptr<Binary>> BinaryOrErr = createBinary(FirstInput);
 
   // We only support regular object files. If failed to open the input as a
   // known binary or this is not an object file use the default binary handler.
   if (errorToBool(BinaryOrErr.takeError()) || !isa<ObjectFile>(*BinaryOrErr))
-    return std::make_unique<BinaryFileHandler>();
+    return new BinaryFileHandler();
 
   // Otherwise create an object file handler. The handler will be owned by the
   // client of this function.
-  return std::make_unique<ObjectFileHandler>(
+  return new ObjectFileHandler(
       std::unique_ptr<ObjectFile>(cast<ObjectFile>(BinaryOrErr->release())));
 }
 
 /// Return an appropriate handler given the input files and options.
-static Expected<std::unique_ptr<FileHandler>>
-CreateFileHandler(MemoryBuffer &FirstInput) {
+static FileHandler *CreateFileHandler(MemoryBuffer &FirstInput) {
   if (FilesType == "i")
-    return std::make_unique<TextFileHandler>(/*Comment=*/"//");
+    return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "ii")
-    return std::make_unique<TextFileHandler>(/*Comment=*/"//");
+    return new TextFileHandler(/*Comment=*/"//");
   if (FilesType == "cui")
-    return std::make_unique<TextFileHandler>(/*Comment=*/"//");
+    return new TextFileHandler(/*Comment=*/"//");
   // TODO: `.d` should be eventually removed once `-M` and its variants are
   // handled properly in offload compilation.
   if (FilesType == "d")
-    return std::make_unique<TextFileHandler>(/*Comment=*/"#");
+    return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "ll")
-    return std::make_unique<TextFileHandler>(/*Comment=*/";");
+    return new TextFileHandler(/*Comment=*/";");
   if (FilesType == "bc")
-    return std::make_unique<BinaryFileHandler>();
+    return new BinaryFileHandler();
   if (FilesType == "s")
-    return std::make_unique<TextFileHandler>(/*Comment=*/"#");
+    return new TextFileHandler(/*Comment=*/"#");
   if (FilesType == "o")
     return CreateObjectFileHandler(FirstInput);
   if (FilesType == "gch")
-    return std::make_unique<BinaryFileHandler>();
+    return new BinaryFileHandler();
   if (FilesType == "ast")
-    return std::make_unique<BinaryFileHandler>();
+    return new BinaryFileHandler();
 
-  return createStringError(errc::invalid_argument,
-                           "'" + FilesType + "': invalid file type specified");
+  errs() << "error: invalid file type specified.\n";
+  return nullptr;
 }
 
 /// Bundle the files. Return true if an error was found.
-static Error BundleFiles() {
+static bool BundleFiles() {
   std::error_code EC;
 
   // Create output file.
   raw_fd_ostream OutputFile(OutputFileNames.front(), EC, sys::fs::OF_None);
-  if (EC)
-    return createFileError(OutputFileNames.front(), EC);
+
+  if (EC) {
+    errs() << "error: Can't open file " << OutputFileNames.front() << ".\n";
+    return true;
+  }
 
   // Open input files.
   SmallVector<std::unique_ptr<MemoryBuffer>, 8u> InputBuffers;
@@ -676,62 +673,61 @@ static Error BundleFiles() {
   for (auto &I : InputFileNames) {
     ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
         MemoryBuffer::getFileOrSTDIN(I);
-    if (std::error_code EC = CodeOrErr.getError())
-      return createFileError(I, EC);
-    InputBuffers.emplace_back(std::move(*CodeOrErr));
+    if (std::error_code EC = CodeOrErr.getError()) {
+      errs() << "error: Can't open file " << I << ": " << EC.message() << "\n";
+      return true;
+    }
+    InputBuffers.emplace_back(std::move(CodeOrErr.get()));
   }
 
   // Get the file handler. We use the host buffer as reference.
   assert(HostInputIndex != ~0u && "Host input index undefined??");
-  Expected<std::unique_ptr<FileHandler>> FileHandlerOrErr =
-      CreateFileHandler(*InputBuffers[HostInputIndex]);
-  if (!FileHandlerOrErr)
-    return FileHandlerOrErr.takeError();
+  std::unique_ptr<FileHandler> FH;
+  FH.reset(CreateFileHandler(*InputBuffers[HostInputIndex].get()));
 
-  std::unique_ptr<FileHandler> &FH = *FileHandlerOrErr;
-  assert(FH);
+  // Quit if we don't have a handler.
+  if (!FH.get())
+    return true;
 
   // Write header.
-  if (Error Err = FH->WriteHeader(OutputFile, InputBuffers))
-    return std::move(Err);
+  FH.get()->WriteHeader(OutputFile, InputBuffers);
 
   // Write all bundles along with the start/end markers. If an error was found
   // writing the end of the bundle component, abort the bundle writing.
   auto Input = InputBuffers.begin();
   for (auto &Triple : TargetNames) {
-    if (Error Err = FH->WriteBundleStart(OutputFile, Triple))
-      return std::move(Err);
-    if (Error Err = FH->WriteBundle(OutputFile, **Input))
-      return std::move(Err);
-    if (Error Err = FH->WriteBundleEnd(OutputFile, Triple))
-      return std::move(Err);
+    FH.get()->WriteBundleStart(OutputFile, Triple);
+    FH.get()->WriteBundle(OutputFile, *Input->get());
+    if (FH.get()->WriteBundleEnd(OutputFile, Triple))
+      return true;
     ++Input;
   }
-  return Error::success();
+  return false;
 }
 
 // Unbundle the files. Return true if an error was found.
-static Error UnbundleFiles() {
+static bool UnbundleFiles() {
   // Open Input file.
   ErrorOr<std::unique_ptr<MemoryBuffer>> CodeOrErr =
       MemoryBuffer::getFileOrSTDIN(InputFileNames.front());
-  if (std::error_code EC = CodeOrErr.getError())
-    return createFileError(InputFileNames.front(), EC);
+  if (std::error_code EC = CodeOrErr.getError()) {
+    errs() << "error: Can't open file " << InputFileNames.front() << ": "
+           << EC.message() << "\n";
+    return true;
+  }
 
-  MemoryBuffer &Input = **CodeOrErr;
+  MemoryBuffer &Input = *CodeOrErr.get();
 
   // Select the right files handler.
-  Expected<std::unique_ptr<FileHandler>> FileHandlerOrErr =
-      CreateFileHandler(Input);
-  if (!FileHandlerOrErr)
-    return FileHandlerOrErr.takeError();
+  std::unique_ptr<FileHandler> FH;
+  FH.reset(CreateFileHandler(Input));
 
-  std::unique_ptr<FileHandler> &FH = *FileHandlerOrErr;
-  assert(FH);
+  // Quit if we don't have a handler.
+  if (!FH.get())
+    return true;
 
   // Read the header of the bundled file.
-  if (Error Err = FH->ReadHeader(Input))
-    return std::move(Err);
+  FH.get()->ReadHeader(Input);
 
   // Create a work list that consist of the map triple/output file.
   StringMap<StringRef> Worklist;
@@ -745,32 +741,29 @@ static Error UnbundleFiles() {
   // assume the file is meant for the host target.
   bool FoundHostBundle = false;
   while (!Worklist.empty()) {
-    Expected<Optional<StringRef>> CurTripleOrErr = FH->ReadBundleStart(Input);
-    if (!CurTripleOrErr)
-      return CurTripleOrErr.takeError();
+    StringRef CurTriple = FH.get()->ReadBundleStart(Input);
 
     // We don't have more bundles.
-    if (!*CurTripleOrErr)
+    if (CurTriple.empty())
       break;
 
-    StringRef CurTriple = **CurTripleOrErr;
-    assert(!CurTriple.empty());
-
     auto Output = Worklist.find(CurTriple);
     // The file may have more bundles for other targets, that we don't care
     // about. Therefore, move on to the next triple
-    if (Output == Worklist.end())
+    if (Output == Worklist.end()) {
       continue;
+    }
 
     // Check if the output file can be opened and copy the bundle to it.
     std::error_code EC;
     raw_fd_ostream OutputFile(Output->second, EC, sys::fs::OF_None);
-    if (EC)
-      return createFileError(Output->second, EC);
-    if (Error Err = FH->ReadBundle(OutputFile, Input))
-      return std::move(Err);
-    if (Error Err = FH->ReadBundleEnd(Input))
-      return std::move(Err);
+    if (EC) {
+      errs() << "error: Can't open file " << Output->second << ": "
+             << EC.message() << "\n";
+      return true;
+    }
+    FH.get()->ReadBundle(OutputFile, Input);
+    FH.get()->ReadBundleEnd(Input);
     Worklist.erase(Output);
 
     // Record if we found the host bundle.
@@ -784,31 +777,38 @@ static Error UnbundleFiles() {
     for (auto &E : Worklist) {
       std::error_code EC;
       raw_fd_ostream OutputFile(E.second, EC, sys::fs::OF_None);
-      if (EC)
-        return createFileError(E.second, EC);
+      if (EC) {
+        errs() << "error: Can't open file " << E.second << ": " << EC.message()
+               << "\n";
+        return true;
+      }
 
       // If this entry has a host kind, copy the input file to the output file.
       if (hasHostKind(E.first()))
         OutputFile.write(Input.getBufferStart(), Input.getBufferSize());
     }
-    return Error::success();
+    return false;
   }
 
   // If we found elements, we emit an error if none of those were for the host
   // in case host bundle name was provided in command line.
-  if (!FoundHostBundle && HostInputIndex != ~0u)
-    return createStringError(inconvertibleErrorCode(),
-                             "Can't find bundle for the host target");
+  if (!FoundHostBundle && HostInputIndex != ~0u) {
+    errs() << "error: Can't find bundle for the host target\n";
+    return true;
+  }
 
   // If we still have any elements in the worklist, create empty files for them.
   for (auto &E : Worklist) {
     std::error_code EC;
     raw_fd_ostream OutputFile(E.second, EC, sys::fs::OF_None);
-    if (EC)
-      return createFileError(E.second, EC);
+    if (EC) {
+      errs() << "error: Can't open file " << E.second << ": "  << EC.message()
+             << "\n";
+      return true;
+    }
   }
 
-  return Error::success();
+  return false;
 }
 
 static void PrintVersion(raw_ostream &OS) {
@@ -832,36 +832,26 @@ int main(int argc, const char **argv) {
     return 0;
   }
 
-  auto reportError = [argv](Error E) {
-    logAllUnhandledErrors(std::move(E), WithColor::error(errs(), argv[0]));
-  };
-
   bool Error = false;
   if (Unbundle) {
     if (InputFileNames.size() != 1) {
       Error = true;
-      reportError(createStringError(
-          errc::invalid_argument,
-          "only one input file supported in unbundling mode"));
+      errs() << "error: only one input file supported in unbundling mode.\n";
     }
     if (OutputFileNames.size() != TargetNames.size()) {
       Error = true;
-      reportError(createStringError(errc::invalid_argument,
-                                    "number of output files and targets should "
-                                    "match in unbundling mode"));
+      errs() << "error: number of output files and targets should match in "
+                "unbundling mode.\n";
     }
   } else {
     if (OutputFileNames.size() != 1) {
       Error = true;
-      reportError(createStringError(
-          errc::invalid_argument,
-          "only one output file supported in bundling mode"));
+      errs() << "error: only one output file supported in bundling mode.\n";
     }
     if (InputFileNames.size() != TargetNames.size()) {
       Error = true;
-      reportError(createStringError(
-          errc::invalid_argument,
-          "number of input files and targets should match in bundling mode"));
+      errs() << "error: number of input files and targets should match in "
+                "bundling mode.\n";
     }
   }
 
@@ -887,15 +877,13 @@ int main(int argc, const char **argv) {
 
     if (!KindIsValid || !TripleIsValid) {
       Error = true;
+      errs() << "error: invalid target '" << Target << "'";
 
-      SmallVector<char, 128u> Buf;
-      raw_svector_ostream Msg(Buf);
-      Msg << "invalid target '" << Target << "'";
       if (!KindIsValid)
-        Msg << ", unknown offloading kind '" << Kind << "'";
+        errs() << ", unknown offloading kind '" << Kind << "'";
       if (!TripleIsValid)
-        Msg << ", unknown target triple '" << Triple << "'";
-      reportError(createStringError(errc::invalid_argument, Msg.str()));
+        errs() << ", unknown target triple '" << Triple << "'";
+      errs() << ".\n";
     }
 
     if (KindIsValid && Kind == "host") {
@@ -911,9 +899,8 @@ int main(int argc, const char **argv) {
   // treat missing host triple as error if we do unbundling.
   if ((Unbundle && HostTargetNum > 1) || (!Unbundle && HostTargetNum != 1)) {
     Error = true;
-    reportError(createStringError(errc::invalid_argument,
-                                  "expecting exactly one host target but got " +
-                                      Twine(HostTargetNum)));
+    errs() << "error: expecting exactly one host target but got "
+           << HostTargetNum << ".\n";
   }
 
   if (Error)
@@ -923,9 +910,5 @@ int main(int argc, const char **argv) {
   // tools.
   BundlerExecutable = sys::fs::getMainExecutable(argv[0], &BundlerExecutable);
 
-  if (llvm::Error Err = Unbundle ? UnbundleFiles() : BundleFiles()) {
-    reportError(std::move(Err));
-    return 1;
-  }
-  return 0;
+  return Unbundle ? UnbundleFiles() : BundleFiles();
 }


        


More information about the cfe-commits mailing list