[PATCH] D72176: Make ErrorList class default constructible and add simple push_back() method

[Stefan Gränitz via Phabricator via llvm-commits]

sgraenitz created this revision.
sgraenitz added reviewers: lhames, mehdi_amini, rnk, vsk.
Herald added a project: LLVM.

ErrorList appears to have some unused potential. It could be a bit easier to use I think. This change proposes the following idiomatic usage. (My actual use-case is still in progress.) Thoughts?

  Error idiomaticUse() {
    ErrorList Errs;
    Errs.push_back(...);
    return make_error<ErrorList>(std::move(Errs));
  }


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72176

Files:
  llvm/include/llvm/Support/Error.h
  llvm/unittests/Support/ErrorTest.cpp


Index: llvm/unittests/Support/ErrorTest.cpp
===================================================================
--- llvm/unittests/Support/ErrorTest.cpp
+++ llvm/unittests/Support/ErrorTest.cpp
@@ -17,6 +17,7 @@
 #include "gtest/gtest-spi.h"
 #include "gtest/gtest.h"
 #include <memory>
+#include <sstream>
 
 using namespace llvm;
 
@@ -367,6 +368,37 @@
   }
 }
 
+// Test direct usage of ErrorList.
+static Error parseInts(std::vector<std::string> Is, void (*H)(int)) {
+  ErrorList Errs;
+  for (const std::string &I : Is) {
+    int Result;
+    std::istringstream SStream(I);
+    SStream >> Result;
+
+    if (SStream.fail()) {
+      Errs.push_back(make_error<StringError>("Failed to parse int: " + I,
+                                             inconvertibleErrorCode()));
+    } else if (!SStream.eof()) {
+      Errs.push_back(
+          make_error<StringError>("Failed to parse full string as int: " + I,
+                                  inconvertibleErrorCode()));
+    } else {
+      H(Result);
+    }
+  }
+  return make_error<ErrorList>(std::move(Errs));
+}
+
+TEST(Error, CheckErrorListIdiomatic) {
+  unsigned Count = 0;
+  Error Err = parseInts({"_", "42", "-0", "1a", "+4", "b5"}, [](int N) {
+    EXPECT_TRUE(N == 42 || N == 0 || N == 4);
+  });
+  handleAllErrors(std::move(Err), [&Count](const StringError &E) { Count++; });
+  EXPECT_EQ(Count, 3u);
+}
+
 // Test that we can consume success values.
 TEST(Error, ConsumeSuccess) {
   Error E = Error::success();
Index: llvm/include/llvm/Support/Error.h
===================================================================
--- llvm/include/llvm/Support/Error.h
+++ llvm/include/llvm/Support/Error.h
@@ -366,6 +366,8 @@
   friend Error joinErrors(Error, Error);
 
 public:
+  ErrorList() = default;
+
   void log(raw_ostream &OS) const override {
     OS << "Multiple errors:\n";
     for (auto &ErrPayload : Payloads) {
@@ -376,6 +378,17 @@
 
   std::error_code convertToErrorCode() const override;
 
+  void push_back(Error Other) {
+    if (Other.isA<ErrorList>()) {
+      auto OtherPayload = Other.takePayload();
+      auto &OtherErrorList = static_cast<ErrorList &>(*OtherPayload);
+      for (std::unique_ptr<ErrorInfoBase> &P : OtherErrorList.Payloads)
+        Payloads.push_back(std::move(P));
+    } else {
+      Payloads.push_back(Other.takePayload());
+    }
+  }
+
   // Used by ErrorInfo::classID.
   static char ID;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D72176.236108.patch
Type: text/x-patch
Size: 2411 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200103/143bf746/attachment.bin>