idmap2: introduce improved Result class
Add a new version of the Result class that functions like the old
Result, but in case of an error, also encodes a string detailing the
error. This will allow us to write the following type of code:
Result<Foo> CreateFoo() {
if (...) {
return Error("errno=%d", errno());
}
return Foo(...);
}
auto foo = CreateFoo();
if (!foo) {
std::cerr << "error: " << foo.GetErrorMessage() << std::endl;
abort();
}
std::cout << "foo=" << *foo << std::endl;
This commit only adds the new Result class. A later change will replace
uses of the old version.
Test: make idmap2_tests
Change-Id: I674d8a06866402adedf85f8514400f25840d5eda
diff --git a/cmds/idmap2/Android.bp b/cmds/idmap2/Android.bp
index 056add5..2fef407 100644
--- a/cmds/idmap2/Android.bp
+++ b/cmds/idmap2/Android.bp
@@ -43,6 +43,7 @@
"libidmap2/PrettyPrintVisitor.cpp",
"libidmap2/RawPrintVisitor.cpp",
"libidmap2/ResourceUtils.cpp",
+ "libidmap2/Result.cpp",
"libidmap2/Xml.cpp",
"libidmap2/ZipFile.cpp",
],
@@ -93,6 +94,7 @@
"tests/PrettyPrintVisitorTests.cpp",
"tests/RawPrintVisitorTests.cpp",
"tests/ResourceUtilsTests.cpp",
+ "tests/ResultTests.cpp",
"tests/XmlTests.cpp",
"tests/ZipFileTests.cpp",
],
diff --git a/cmds/idmap2/include/idmap2/Result.h b/cmds/idmap2/include/idmap2/Result.h
index 6189ea3..d88dd51 100644
--- a/cmds/idmap2/include/idmap2/Result.h
+++ b/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 @@
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_
diff --git a/cmds/idmap2/libidmap2/Result.cpp b/cmds/idmap2/libidmap2/Result.cpp
new file mode 100644
index 0000000..a5c9999
--- /dev/null
+++ b/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
diff --git a/cmds/idmap2/static-checks.sh b/cmds/idmap2/static-checks.sh
index ad9830b..5740710 100755
--- a/cmds/idmap2/static-checks.sh
+++ b/cmds/idmap2/static-checks.sh
@@ -73,6 +73,7 @@
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"
diff --git a/cmds/idmap2/tests/ResultTests.cpp b/cmds/idmap2/tests/ResultTests.cpp
new file mode 100644
index 0000000..d82f0c4
--- /dev/null
+++ b/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