[llvm] r276430 - [Support] Make ErrorAsOutParameter take an Error* rather than an Error&.

Lang Hames via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 22 09:11:25 PDT 2016


Author: lhames
Date: Fri Jul 22 11:11:25 2016
New Revision: 276430

URL: http://llvm.org/viewvc/llvm-project?rev=276430&view=rev
Log:
[Support] Make ErrorAsOutParameter take an Error* rather than an Error&.

This allows ErrorAsOutParameter to work better with "optional" errors. For
example, consider a function where for certain input values it is known that
the function can't fail. This can now be written as:

Result foo(Arg X, Error *Err) {
  ErrorAsOutParameter EAO(Err);

  if (<Error Condition>) {
    if (Err)
      *Err = <report error>;
    else
      llvm_unreachable("Unexpected failure!");
  }
}

Rather than having to construct an ErrorAsOutParameter under every conditional
where Err is known to be non-null.


Modified:
    llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
    llvm/trunk/include/llvm/Object/Archive.h
    llvm/trunk/include/llvm/Support/Error.h
    llvm/trunk/lib/Object/Archive.cpp
    llvm/trunk/lib/Object/MachOObjectFile.cpp
    llvm/trunk/lib/Object/MachOUniversal.cpp
    llvm/trunk/unittests/Support/ErrorTest.cpp

Modified: llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h (original)
+++ llvm/trunk/include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h Fri Jul 22 11:11:25 2016
@@ -688,7 +688,7 @@ public:
 
 private:
   OrcRemoteTargetClient(ChannelT &Channel, Error &Err) : Channel(Channel) {
-    ErrorAsOutParameter EAO(Err);
+    ErrorAsOutParameter EAO(&Err);
     if (auto RIOrErr = callST<GetRemoteInfo>(Channel)) {
       std::tie(RemoteTargetTriple, RemotePointerSize, RemotePageSize,
                RemoteTrampolineSize, RemoteIndirectStubSize) = *RIOrErr;

Modified: llvm/trunk/include/llvm/Object/Archive.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Object/Archive.h?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Object/Archive.h (original)
+++ llvm/trunk/include/llvm/Object/Archive.h Fri Jul 22 11:11:25 2016
@@ -131,10 +131,10 @@ public:
     // iteration.  And if there is an error break out of the loop.
     child_iterator &operator++() { // Preincrement
       assert(E && "Can't increment iterator with no Error attached");
+      ErrorAsOutParameter ErrAsOutParam(E);
       if (auto ChildOrErr = C.getNext())
         C = *ChildOrErr;
       else {
-        ErrorAsOutParameter ErrAsOutParam(*E);
         C = C.getParent()->child_end().C;
         *E = ChildOrErr.takeError();
         E = nullptr;

Modified: llvm/trunk/include/llvm/Support/Error.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/Error.h?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Support/Error.h (original)
+++ llvm/trunk/include/llvm/Support/Error.h Fri Jul 22 11:11:25 2016
@@ -571,24 +571,32 @@ inline void consumeError(Error Err) {
 /// RAII:
 ///
 /// Result foo(Error &Err) {
-///   ErrorAsOutParameter ErrAsOutParam(Err); // 'Checked' flag set
+///   ErrorAsOutParameter ErrAsOutParam(&Err); // 'Checked' flag set
 ///   // <body of foo>
 ///   // <- 'Checked' flag auto-cleared when ErrAsOutParam is destructed.
 /// }
+///
+/// ErrorAsOutParameter takes an Error* rather than Error& so that it can be
+/// used with optional Errors (Error pointers that are allowed to be null). If
+/// ErrorAsOutParameter took an Error reference, an instance would have to be
+/// created inside every condition that verified that Error was non-null. By
+/// taking an Error pointer we can just create one instance at the top of the
+/// function.
 class ErrorAsOutParameter {
 public:
-  ErrorAsOutParameter(Error &Err) : Err(Err) {
+  ErrorAsOutParameter(Error *Err) : Err(Err) {
     // Raise the checked bit if Err is success.
-    (void)!!Err;
+    if (Err)
+      (void)!!*Err;
   }
   ~ErrorAsOutParameter() {
     // Clear the checked bit.
-    if (!Err)
-      Err = Error::success();
+    if (Err && !*Err)
+      *Err = Error::success();
   }
 
 private:
-  Error &Err;
+  Error *Err;
 };
 
 /// Tagged union holding either a T or a Error.

Modified: llvm/trunk/lib/Object/Archive.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/Archive.cpp?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/lib/Object/Archive.cpp (original)
+++ llvm/trunk/lib/Object/Archive.cpp Fri Jul 22 11:11:25 2016
@@ -108,16 +108,15 @@ Archive::Child::Child(const Archive *Par
     : Parent(Parent) {
   if (!Start)
     return;
+  ErrorAsOutParameter ErrAsOutParam(Err);
 
   uint64_t Size = sizeof(ArchiveMemberHeader);
   Data = StringRef(Start, Size);
   if (!isThinMember()) {
     Expected<uint64_t> MemberSize = getRawSize();
     if (!MemberSize) {
-      if (Err) {
-        ErrorAsOutParameter ErrAsOutParam(*Err);
+      if (Err)
         *Err = MemberSize.takeError();
-      }
       return;
     }
     Size += MemberSize.get();
@@ -299,7 +298,7 @@ void Archive::setFirstRegular(const Chil
 
 Archive::Archive(MemoryBufferRef Source, Error &Err)
     : Binary(Binary::ID_Archive, Source) {
-  ErrorAsOutParameter ErrAsOutParam(Err);
+  ErrorAsOutParameter ErrAsOutParam(&Err);
   StringRef Buffer = Data.getBuffer();
   // Check for sufficient magic.
   if (Buffer.startswith(ThinMagic)) {

Modified: llvm/trunk/lib/Object/MachOObjectFile.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOObjectFile.cpp?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOObjectFile.cpp (original)
+++ llvm/trunk/lib/Object/MachOObjectFile.cpp Fri Jul 22 11:11:25 2016
@@ -267,7 +267,7 @@ MachOObjectFile::MachOObjectFile(MemoryB
       DataInCodeLoadCmd(nullptr), LinkOptHintsLoadCmd(nullptr),
       DyldInfoLoadCmd(nullptr), UuidLoadCmd(nullptr),
       HasPageZeroSegment(false) {
-  ErrorAsOutParameter ErrAsOutParam(Err);
+  ErrorAsOutParameter ErrAsOutParam(&Err);
   uint64_t BigSize;
   if (is64Bit()) {
     parseHeader(this, Header64, Err);

Modified: llvm/trunk/lib/Object/MachOUniversal.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Object/MachOUniversal.cpp?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/lib/Object/MachOUniversal.cpp (original)
+++ llvm/trunk/lib/Object/MachOUniversal.cpp Fri Jul 22 11:11:25 2016
@@ -114,7 +114,7 @@ MachOUniversalBinary::create(MemoryBuffe
 MachOUniversalBinary::MachOUniversalBinary(MemoryBufferRef Source, Error &Err)
     : Binary(Binary::ID_MachOUniversalBinary, Source), Magic(0),
       NumberOfObjects(0) {
-  ErrorAsOutParameter ErrAsOutParam(Err);
+  ErrorAsOutParameter ErrAsOutParam(&Err);
   if (Data.getBufferSize() < sizeof(MachO::fat_header)) {
     Err = make_error<GenericBinaryError>("File too small to be a Mach-O "
                                          "universal file",

Modified: llvm/trunk/unittests/Support/ErrorTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/ErrorTest.cpp?rev=276430&r1=276429&r2=276430&view=diff
==============================================================================
--- llvm/trunk/unittests/Support/ErrorTest.cpp (original)
+++ llvm/trunk/unittests/Support/ErrorTest.cpp Fri Jul 22 11:11:25 2016
@@ -109,7 +109,7 @@ TEST(Error, UncheckedSuccess) {
 
 // ErrorAsOutParameter tester.
 void errAsOutParamHelper(Error &Err) {
-  ErrorAsOutParameter ErrAsOutParam(Err);
+  ErrorAsOutParameter ErrAsOutParam(&Err);
   // Verify that checked flag is raised - assignment should not crash.
   Err = Error::success();
   // Raise the checked bit manually - caller should still have to test the




More information about the llvm-commits mailing list