Merge "idmap2: introduce improved Result class"

cmds/idmap2/Android.bp

@@ -43,6 +43,7 @@ cc_library {
         "libidmap2/PrettyPrintVisitor.cpp",
         "libidmap2/RawPrintVisitor.cpp",
         "libidmap2/ResourceUtils.cpp",
+        "libidmap2/Result.cpp",
         "libidmap2/Xml.cpp",
         "libidmap2/ZipFile.cpp",
     ],
@@ -93,6 +94,7 @@ cc_test {
         "tests/PrettyPrintVisitorTests.cpp",
         "tests/RawPrintVisitorTests.cpp",
         "tests/ResourceUtilsTests.cpp",
+        "tests/ResultTests.cpp",
         "tests/XmlTests.cpp",
         "tests/ZipFileTests.cpp",
     ],

cmds/idmap2/include/idmap2/Result.h

@@ -18,6 +18,11 @@
 #define IDMAP2_INCLUDE_IDMAP2_RESULT_H_
 
 #include <optional>
+#include <string>
+#include <utility>
+#include <variant>
+
+#include "android-base/logging.h"  // CHECK
 
 namespace android::idmap2 {
 
@@ -26,6 +31,125 @@ using Result = std::optional<T>;
 
 static constexpr std::nullopt_t kResultError = std::nullopt;
 
+namespace v2 {
+
+using Unit = std::monostate;
+
+class Error {
+ public:
+  explicit Error(const Error& parent) = default;
+
+  // NOLINTNEXTLINE(cert-dcl50-cpp)
+  explicit Error(const char* fmt, ...) __attribute__((__format__(printf, 2, 3)));
+
+  // NOLINTNEXTLINE(cert-dcl50-cpp)
+  explicit Error(const Error& parent, const char* fmt, ...)
+      __attribute__((__format__(printf, 3, 4)));
+
+  inline std::string GetMessage() const {
+    return msg_;
+  }
+
+ private:
+  std::string msg_;
+};
+
+template <typename T>
+class Result {
+ public:
+  Result(const T& value);      // NOLINT(runtime/explicit)
+  Result(T&& value) noexcept;  // NOLINT(runtime/explicit)
+
+  Result(const Error& error);      // NOLINT(runtime/explicit)
+  Result(Error&& error) noexcept;  // NOLINT(runtime/explicit)
+
+  Result(const Result& error) = default;
+
+  Result& operator=(const Result& rhs) = default;
+  Result& operator=(Result&& rhs) noexcept = default;
+
+  explicit operator bool() const;
+
+  constexpr const T& operator*() const&;
+  T& operator*() &;
+
+  constexpr const T* operator->() const&;
+  T* operator->() &;
+
+  std::string GetErrorMessage() const;
+  Error GetError() const;
+
+ private:
+  bool is_ok() const;
+
+  std::variant<T, Error> data_;
+};
+
+template <typename T>
+Result<T>::Result(const T& value) : data_(std::in_place_type<T>, value) {
+}
+
+template <typename T>
+Result<T>::Result(T&& value) noexcept : data_(std::in_place_type<T>, std::forward<T>(value)) {
+}
+
+template <typename T>
+Result<T>::Result(const Error& error) : data_(std::in_place_type<Error>, error) {
+}
+
+template <typename T>
+Result<T>::Result(Error&& error) noexcept
+    : data_(std::in_place_type<Error>, std::forward<Error>(error)) {
+}
+
+template <typename T>
+Result<T>::operator bool() const {
+  return is_ok();
+}
+
+template <typename T>
+constexpr const T& Result<T>::operator*() const& {
+  CHECK(is_ok()) << "Result<T>::operator* called in ERROR state";
+  return std::get<T>(data_);
+}
+
+template <typename T>
+T& Result<T>::operator*() & {
+  CHECK(is_ok()) << "Result<T>::operator* called in ERROR state";
+  return std::get<T>(data_);
+}
+
+template <typename T>
+constexpr const T* Result<T>::operator->() const& {
+  CHECK(is_ok()) << "Result<T>::operator-> called in ERROR state";
+  return &std::get<T>(data_);
+}
+
+template <typename T>
+T* Result<T>::operator->() & {
+  CHECK(is_ok()) << "Result<T>::operator-> called in ERROR state";
+  return &std::get<T>(data_);
+}
+
+template <typename T>
+inline std::string Result<T>::GetErrorMessage() const {
+  CHECK(!is_ok()) << "Result<T>::GetErrorMessage called in OK state";
+  return std::get<Error>(data_).GetMessage();
+}
+
+template <typename T>
+inline Error Result<T>::GetError() const {
+  CHECK(!is_ok()) << "Result<T>::GetError called in OK state";
+  return Error(std::get<Error>(data_));
+}
+
+template <typename T>
+inline bool Result<T>::is_ok() const {
+  return std::holds_alternative<T>(data_);
+}
+
+}  // namespace v2
+
 }  // namespace android::idmap2
 
 #endif  // IDMAP2_INCLUDE_IDMAP2_RESULT_H_

cmds/idmap2/libidmap2/Result.cpp

@@ -0,0 +1,41 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <cstdarg>
+
+#include "android-base/stringprintf.h"
+
+#include "idmap2/Result.h"
+
+namespace android::idmap2 {
+
+v2::Error::Error(const char* fmt, ...) {
+  va_list ap;
+  va_start(ap, fmt);
+  base::StringAppendV(&msg_, fmt, ap);
+  va_end(ap);
+}
+
+v2::Error::Error(const Error& parent, const char* fmt, ...) : msg_(parent.msg_) {
+  msg_.append(" -> ");
+
+  va_list ap;
+  va_start(ap, fmt);
+  base::StringAppendV(&msg_, fmt, ap);
+  va_end(ap);
+}
+
+}  // namespace android::idmap2

cmds/idmap2/static-checks.sh

@@ -73,6 +73,7 @@ function _cpplint()
     local output="$($cpplint --quiet $cpp_files 2>&1 >/dev/null | grep -v \
         -e 'Found C system header after C++ system header.' \
         -e 'Unknown NOLINT error category: misc-non-private-member-variables-in-classes' \
+        -e 'Unknown NOLINT error category: performance-unnecessary-copy-initialization' \
     )"
     if [[ "$output" ]]; then
         echo "$output"

cmds/idmap2/tests/ResultTests.cpp

@@ -0,0 +1,288 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <memory>
+#include <type_traits>
+#include <utility>
+
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include "idmap2/Result.h"
+
+namespace android::idmap2 {
+
+struct Container {
+  uint32_t value;  // NOLINT(misc-non-private-member-variables-in-classes)
+};
+
+// Tests: Error
+
+TEST(ResultTests, ErrorTraits) {
+  ASSERT_TRUE(std::is_move_constructible<v2::Error>::value);
+  ASSERT_TRUE(std::is_move_assignable<v2::Error>::value);
+  ASSERT_TRUE(std::is_copy_constructible<v2::Error>::value);
+  ASSERT_TRUE(std::is_copy_assignable<v2::Error>::value);
+}
+
+TEST(ResultTests, ErrorCtorFormat) {
+  v2::Error e("%s=0x%08x", "resid", 0x7f010002);
+  ASSERT_EQ(e.GetMessage(), "resid=0x7f010002");
+}
+
+TEST(ResultTests, ErrorPropagateParent) {
+  v2::Error e1("foo");
+  ASSERT_EQ(e1.GetMessage(), "foo");
+
+  v2::Error e2(e1, "bar");
+  ASSERT_EQ(e2.GetMessage(), "foo -> bar");
+
+  v2::Error e3(e2);  // NOLINT(performance-unnecessary-copy-initialization)
+  ASSERT_EQ(e3.GetMessage(), "foo -> bar");
+
+  v2::Error e4(e3, "%02d", 1);
+  ASSERT_EQ(e4.GetMessage(), "foo -> bar -> 01");
+}
+
+// Tests: Result<T> member functions
+
+// Result(const Result&)
+TEST(ResultTests, CopyConstructor) {
+  v2::Result<uint32_t> r1(42U);
+
+  v2::Result<uint32_t> r2(r1);
+  ASSERT_TRUE(r2);
+  ASSERT_EQ(*r2, 42U);
+
+  v2::Result<uint32_t> r3 = r2;
+  ASSERT_TRUE(r3);
+  ASSERT_EQ(*r3, 42U);
+}
+
+// Result(const T&)
+TEST(ResultTests, Constructor) {
+  uint32_t v = 42U;
+  v2::Result<uint32_t> r1(v);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(*r1, 42U);
+
+  v2::Error e("foo");
+  v2::Result<uint32_t> r2(e);
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+}
+
+// Result(const T&&)
+TEST(ResultTests, MoveConstructor) {
+  v2::Result<uint32_t> r1(42U);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(*r1, 42U);
+
+  v2::Result<uint32_t> r2(v2::Error("foo"));
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+}
+
+// operator=
+TEST(ResultTests, CopyAssignmentOperator) {
+  // note: 'Result<...> r2 = r1;' calls the copy ctor
+  v2::Result<uint32_t> r1(42U);
+  v2::Result<uint32_t> r2(0U);
+  r2 = r1;
+  ASSERT_TRUE(r2);
+  ASSERT_EQ(*r2, 42U);
+
+  v2::Result<uint32_t> r3(v2::Error("foo"));
+  r2 = r3;
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+}
+
+TEST(ResultTests, MoveAssignmentOperator) {
+  v2::Result<uint32_t> r(0U);
+  r = v2::Result<uint32_t>(42U);
+  ASSERT_TRUE(r);
+  ASSERT_EQ(*r, 42U);
+
+  r = v2::Result<uint32_t>(v2::Error("foo"));
+  ASSERT_FALSE(r);
+  ASSERT_EQ(r.GetErrorMessage(), "foo");
+}
+
+// operator bool()
+TEST(ResultTests, BoolOperator) {
+  v2::Result<uint32_t> r1(42U);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(*r1, 42U);
+
+  v2::Result<uint32_t> r2(v2::Error("foo"));
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+}
+
+// operator*
+TEST(ResultTests, IndirectionOperator) {
+  const v2::Result<uint32_t> r1(42U);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(*r1, 42U);
+
+  const v2::Result<Container> r2(Container{42U});
+  ASSERT_TRUE(r2);
+  const Container& c = *r2;
+  ASSERT_EQ(c.value, 42U);
+
+  v2::Result<Container> r3(Container{42U});
+  ASSERT_TRUE(r3);
+  ASSERT_EQ((*r3).value, 42U);
+  (*r3).value = 0U;
+  ASSERT_EQ((*r3).value, 0U);
+}
+
+// operator->
+TEST(ResultTests, DereferenceOperator) {
+  const v2::Result<Container> r1(Container{42U});
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(r1->value, 42U);
+
+  v2::Result<Container> r2(Container{42U});
+  ASSERT_TRUE(r2);
+  ASSERT_EQ(r2->value, 42U);
+  r2->value = 0U;
+  ASSERT_EQ(r2->value, 0U);
+}
+
+// Tests: intended use of Result<T>
+
+TEST(ResultTests, ResultTraits) {
+  ASSERT_TRUE(std::is_move_constructible<v2::Result<uint32_t>>::value);
+  ASSERT_TRUE(std::is_move_assignable<v2::Result<uint32_t>>::value);
+  ASSERT_TRUE(std::is_copy_constructible<v2::Result<uint32_t>>::value);
+  ASSERT_TRUE(std::is_copy_assignable<v2::Result<uint32_t>>::value);
+}
+
+TEST(ResultTests, UnitTypeResult) {
+  v2::Result<v2::Unit> r(v2::Unit{});
+  ASSERT_TRUE(r);
+}
+
+struct RefCountData {
+  int ctor;       // NOLINT(misc-non-private-member-variables-in-classes)
+  int copy_ctor;  // NOLINT(misc-non-private-member-variables-in-classes)
+  int dtor;       // NOLINT(misc-non-private-member-variables-in-classes)
+  int move;       // NOLINT(misc-non-private-member-variables-in-classes)
+};
+
+class RefCountContainer {
+ public:
+  explicit RefCountContainer(RefCountData& data) : data_(data) {
+    ++data_.ctor;
+  }
+
+  RefCountContainer(RefCountContainer const&) = delete;
+
+  RefCountContainer(RefCountContainer&& rhs) noexcept : data_(rhs.data_) {
+    ++data_.copy_ctor;
+  }
+
+  RefCountContainer& operator=(RefCountContainer const&) = delete;
+
+  RefCountContainer& operator=(RefCountContainer&& rhs) noexcept {
+    data_ = rhs.data_;
+    ++data_.move;
+    return *this;
+  }
+
+  ~RefCountContainer() {
+    ++data_.dtor;
+  }
+
+ private:
+  RefCountData& data_;
+};
+
+TEST(ResultTests, ReferenceCount) {
+  ASSERT_TRUE(std::is_move_constructible<RefCountContainer>::value);
+  ASSERT_TRUE(std::is_move_assignable<RefCountContainer>::value);
+  ASSERT_FALSE(std::is_copy_constructible<RefCountContainer>::value);
+  ASSERT_FALSE(std::is_copy_assignable<RefCountContainer>::value);
+
+  RefCountData rc{0, 0, 0, 0};
+  { v2::Result<RefCountContainer> r(RefCountContainer{rc}); }
+  ASSERT_EQ(rc.ctor, 1);
+  ASSERT_EQ(rc.copy_ctor, 1);
+  ASSERT_EQ(rc.move, 0);
+  ASSERT_EQ(rc.dtor, 2);
+}
+
+v2::Result<Container> CreateContainer(bool succeed) {
+  if (!succeed) {
+    return v2::Error("foo");
+  }
+  return Container{42U};
+}
+
+TEST(ResultTests, FunctionReturn) {
+  auto r1 = CreateContainer(true);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ(r1->value, 42U);
+
+  auto r2 = CreateContainer(false);
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+  ASSERT_EQ(r2.GetError().GetMessage(), "foo");
+}
+
+v2::Result<Container> FailToCreateContainer() {
+  auto container = CreateContainer(false);
+  if (!container) {
+    return v2::Error(container.GetError(), "bar");
+  }
+  return container;
+}
+
+TEST(ResultTests, CascadeError) {
+  auto container = FailToCreateContainer();
+  ASSERT_FALSE(container);
+  ASSERT_EQ(container.GetErrorMessage(), "foo -> bar");
+}
+
+struct NoCopyContainer {
+  uint32_t value;  // NOLINT(misc-non-private-member-variables-in-classes)
+  DISALLOW_COPY_AND_ASSIGN(NoCopyContainer);
+};
+
+v2::Result<std::unique_ptr<NoCopyContainer>> CreateNoCopyContainer(bool succeed) {
+  if (!succeed) {
+    return v2::Error("foo");
+  }
+  std::unique_ptr<NoCopyContainer> p(new NoCopyContainer{0U});
+  p->value = 42U;
+  return std::move(p);
+}
+
+TEST(ResultTests, UniquePtr) {
+  auto r1 = CreateNoCopyContainer(true);
+  ASSERT_TRUE(r1);
+  ASSERT_EQ((*r1)->value, 42U);
+  (*r1)->value = 0U;
+  ASSERT_EQ((*r1)->value, 0U);
+
+  auto r2 = CreateNoCopyContainer(false);
+  ASSERT_FALSE(r2);
+  ASSERT_EQ(r2.GetErrorMessage(), "foo");
+}
+
+}  // namespace android::idmap2