This is an automated email from the ASF dual-hosted git repository.
tison pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/incubator-kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 4ba6b10 Add `StatusOr` for error handling in modern C++ style (#768)
4ba6b10 is described below
commit 4ba6b100ca62044cae9426f02c9e358b8f56841a
Author: Twice <[email protected]>
AuthorDate: Tue Aug 16 22:37:06 2022 +0800
Add `StatusOr` for error handling in modern C++ style (#768)
---
src/redis_cmd.cc | 2 +-
src/redis_connection.cc | 2 +-
src/status.h | 211 ++++++++++++++++++++++++++++++++++++++++---
tests/cppunit/status_test.cc | 146 ++++++++++++++++++++++++++++++
4 files changed, 348 insertions(+), 13 deletions(-)
diff --git a/src/redis_cmd.cc b/src/redis_cmd.cc
index 7437635..6538a98 100644
--- a/src/redis_cmd.cc
+++ b/src/redis_cmd.cc
@@ -118,7 +118,7 @@ class CommandNamespace : public Commander {
} else {
std::string token;
auto s = config->GetNamespace(args_[2], &token);
- if (s.IsNotFound()) {
+ if (s.Is<Status::NotFound>()) {
*output = Redis::NilString();
} else {
*output = Redis::BulkString(token);
diff --git a/src/redis_connection.cc b/src/redis_connection.cc
index 67a81d4..35a276a 100644
--- a/src/redis_connection.cc
+++ b/src/redis_connection.cc
@@ -429,7 +429,7 @@ void Connection::ExecuteCommands(std::deque<CommandTokens>
*to_process_cmds) {
// Break the execution loop when occurring the blocking command like BLPOP
or BRPOP,
// it will suspend the connection and wait for the wakeup signal.
- if (s.IsBlockingCommand()) {
+ if (s.Is<Status::BlockingCmd>()) {
break;
}
// Reply for MULTI
diff --git a/src/status.h b/src/status.h
index 21ae3c4..5ec3f5f 100644
--- a/src/status.h
+++ b/src/status.h
@@ -21,12 +21,17 @@
#pragma once
#include <string>
+#include <type_traits>
#include <utility>
+#include <memory>
+#include <algorithm>
+#include <tuple>
+#include <glog/logging.h>
class Status {
public:
- enum Code {
- cOK,
+ enum Code : unsigned char {
+ cOK = 0,
NotOK,
NotFound,
@@ -61,20 +66,204 @@ class Status {
};
Status() : Status(cOK) {}
- explicit Status(Code code, std::string msg = {}) : code_(code),
msg_(std::move(msg)) {}
- bool IsOK() { return code_ == cOK; }
- bool IsNotFound() { return code_ == NotFound; }
- bool IsImorting() { return code_ == SlotImport; }
- bool IsBlockingCommand() { return code_ == BlockingCmd; }
- std::string Msg() {
- if (IsOK()) {
- return "ok";
- }
+ explicit Status(Code code, std::string msg = {})
+ : code_(code), msg_(std::move(msg)) {}
+
+ template <Code code>
+ bool Is() const { return code_ == code; }
+
+ bool IsOK() const { return Is<cOK>(); }
+ operator bool() const { return IsOK(); }
+
+ Code GetCode() const {
+ return code_;
+ }
+
+ std::string Msg() const& {
+ if (*this) return ok_msg();
return msg_;
}
+
+ std::string Msg() && {
+ if (*this) return ok_msg();
+ return std::move(msg_);
+ }
+
static Status OK() { return {}; }
private:
Code code_;
std::string msg_;
+
+ static constexpr const char* ok_msg() {
+ return "ok";
+ }
+
+ template <typename T>
+ friend struct StatusOr;
+};
+
+template <typename ...Ts>
+using first_element = typename std::tuple_element<0, std::tuple<Ts...>>::type;
+
+template <typename T>
+using remove_cvref_t = typename std::remove_cv<typename
std::remove_reference<T>::type>::type;
+
+template <typename T>
+struct StatusOr;
+
+template <typename T>
+struct IsStatusOr : std::integral_constant<bool, false> {};
+
+template <typename T>
+struct IsStatusOr<StatusOr<T>> : std::integral_constant<bool, true> {};
+
+template <typename T>
+struct StatusOr {
+ static_assert(!std::is_same<T, Status>::value, "value_type cannot be
Status");
+ static_assert(!std::is_same<T, Status::Code>::value, "value_type cannot be
Status::Code");
+ static_assert(!IsStatusOr<T>::value, "value_type cannot be StatusOr");
+ static_assert(!std::is_reference<T>::value, "value_type cannot be
reference");
+
+ using value_type = T;
+
+ // we use std::unique_ptr to make the error part as small as enough
+ using error_type = std::unique_ptr<std::string>;
+
+ using Code = Status::Code;
+
+ explicit StatusOr(Status s) : code_(s.code_) {
+ CHECK(!s);
+ new(value_or_error_) error_type(new std::string(std::move(s.msg_)));
+ }
+
+ StatusOr(Code code, std::string msg = {}) : code_(code) { // NOLINT
+ CHECK(code != Code::cOK);
+ new(value_or_error_) error_type(new std::string(std::move(msg)));
+ }
+
+ template <typename ...Ts,
+ typename std::enable_if<
+ (sizeof...(Ts) > 0 &&
+ !std::is_same<Status, remove_cvref_t<first_element<Ts...>>>::value &&
+ !std::is_same<Code, remove_cvref_t<first_element<Ts...>>>::value &&
+ !std::is_same<value_type, remove_cvref_t<first_element<Ts...>>>::value
&&
+ !std::is_same<StatusOr, remove_cvref_t<first_element<Ts...>>>::value
+ ), int>::type = 0> // NOLINT
+ explicit StatusOr(Ts && ... args) : code_(Code::cOK) {
+ new(value_or_error_) value_type(std::forward<Ts>(args)...);
+ }
+
+ StatusOr(T&& value) : code_(Code::cOK) { // NOLINT
+ new(value_or_error_) value_type(std::move(value));
+ }
+
+ StatusOr(const T& value) : code_(Code::cOK) { // NOLINT
+ new(value_or_error_) value_type(value);
+ }
+
+ StatusOr(const StatusOr&) = delete;
+ StatusOr(StatusOr&& other) : code_(other.code_) {
+ if (code_ == Code::cOK) {
+ new(value_or_error_) value_type(std::move(other.getValue()));
+ } else {
+ new(value_or_error_) error_type(std::move(other.getError()));
+ }
+ }
+
+ Status& operator=(const Status&) = delete;
+
+ template <Code code>
+ bool Is() const { return code_ == code; }
+
+ bool IsOK() const { return Is<Code::cOK>(); }
+ operator bool() const { return IsOK(); }
+
+ Status ToStatus() const& {
+ if (*this) return Status::OK();
+ return Status(code_, *getError());
+ }
+
+ Status ToStatus() && {
+ if (*this) return Status::OK();
+ return Status(code_, std::move(*getError()));
+ }
+
+ Code GetCode() const {
+ return code_;
+ }
+
+ value_type& GetValue() & {
+ CHECK(*this);
+ return getValue();
+ }
+
+ value_type&& GetValue() && {
+ CHECK(*this);
+ return std::move(getValue());
+ }
+
+ const value_type& GetValue() const& {
+ CHECK(*this);
+ return getValue();
+ }
+
+ value_type& operator*() & {
+ return GetValue();
+ }
+
+ value_type&& operator*() && {
+ return std::move(GetValue());
+ }
+
+ const value_type& operator*() const& {
+ return GetValue();
+ }
+
+ value_type* operator->() {
+ return &GetValue();
+ }
+
+ const value_type* operator->() const {
+ return &GetValue();
+ }
+
+ std::string Msg() const& {
+ if (*this) return Status::ok_msg();
+ return *getError();
+ }
+
+ std::string Msg() && {
+ if (*this) return Status::ok_msg();
+ return std::move(*getError());
+ }
+
+ ~StatusOr() {
+ if (*this) {
+ getValue().~value_type();
+ } else {
+ getError().~error_type();
+ }
+ }
+
+ private:
+ Status::Code code_;
+ alignas(value_type) alignas(error_type) unsigned char value_or_error_
+ [sizeof(value_type) < sizeof(error_type) ? sizeof(error_type) :
sizeof(value_type)];
+
+ value_type& getValue() {
+ return *reinterpret_cast<value_type*>(value_or_error_);
+ }
+
+ const value_type& getValue() const {
+ return *reinterpret_cast<const value_type*>(value_or_error_);
+ }
+
+ error_type& getError() {
+ return *reinterpret_cast<error_type*>(value_or_error_);
+ }
+
+ const error_type& getError() const {
+ return *reinterpret_cast<const error_type*>(value_or_error_);
+ }
};
diff --git a/tests/cppunit/status_test.cc b/tests/cppunit/status_test.cc
new file mode 100644
index 0000000..dac3683
--- /dev/null
+++ b/tests/cppunit/status_test.cc
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 <gtest/gtest.h>
+
+#include <memory>
+#include <status.h>
+
+TEST(StatusOr, Scalar) {
+ auto f = [](int x) -> StatusOr<int> {
+ if (x > 10) {
+ return {Status::NotOK, "x large than 10"};
+ }
+
+ return 2 * x + 5;
+ };
+
+ ASSERT_EQ(*f(1), 7);
+ ASSERT_EQ(*f(5), 15);
+ ASSERT_EQ(f(7).GetValue(), 19);
+ ASSERT_EQ(f(7).GetCode(), Status::cOK);
+ ASSERT_EQ(f(7).Msg(), "ok");
+ ASSERT_TRUE(f(6));
+ ASSERT_EQ(f(11).GetCode(), Status::NotOK);
+ ASSERT_EQ(f(11).Msg(), "x large than 10");
+ ASSERT_FALSE(f(12));
+
+ auto x = f(5);
+ ASSERT_EQ(*x, 15);
+ ASSERT_EQ(x.Msg(), "ok");
+ ASSERT_EQ(x.GetValue(), 15);
+ ASSERT_EQ(x.GetCode(), Status::cOK);
+
+ auto y = f(11);
+ ASSERT_EQ(y.Msg(), "x large than 10");
+ ASSERT_EQ(y.GetCode(), Status::NotOK);
+
+ auto g = [f](int y) -> StatusOr<int> {
+ if (y > 5 && y < 15) {
+ return {Status::NotOK, "y large than 5"};
+ }
+
+ auto res = f(y);
+ if (!res) return res;
+
+ return *res * 10;
+ };
+
+ ASSERT_EQ(*g(1), 70);
+ ASSERT_EQ(*g(5), 150);
+ ASSERT_EQ(g(1).GetValue(), 70);
+ ASSERT_EQ(g(1).GetCode(), Status::cOK);
+ ASSERT_EQ(g(1).Msg(), "ok");
+ ASSERT_EQ(g(6).GetCode(), Status::NotOK);
+ ASSERT_EQ(g(6).Msg(), "y large than 5");
+ ASSERT_EQ(g(20).GetCode(), Status::NotOK);
+ ASSERT_EQ(g(20).Msg(), "x large than 10");
+ ASSERT_EQ(g(11).GetCode(), Status::NotOK);
+ ASSERT_EQ(g(11).Msg(), "y large than 5");
+}
+
+TEST(StatusOr, String) {
+ auto f = [](std::string x) -> StatusOr<std::string> {
+ if (x.size() > 10) {
+ return {Status::NotOK, "string too long"};
+ }
+
+ return x + " hello";
+ };
+
+ auto g = [f](std::string x) -> StatusOr<std::string> {
+ if (x.size() < 5) {
+ return {Status::NotOK, "string too short"};
+ }
+
+ auto res = f(x);
+ if (!res) return res;
+
+ return "hi " + *res;
+ };
+
+ ASSERT_TRUE(f("1"));
+ ASSERT_FALSE(f("12345678901"));
+ ASSERT_FALSE(g("1"));
+
+ ASSERT_EQ(*f("twice"), "twice hello");
+ ASSERT_EQ(*g("twice"), "hi twice hello");
+ ASSERT_EQ(g("shrt").GetCode(), Status::NotOK);
+ ASSERT_EQ(g("shrt").Msg(), "string too short");
+ ASSERT_EQ(g("loooooooooooog").GetCode(), Status::NotOK);
+ ASSERT_EQ(g("loooooooooooog").Msg(), "string too long");
+
+ ASSERT_EQ(g("twice").ToStatus().GetCode(), Status::cOK);
+ ASSERT_EQ(g("").ToStatus().GetCode(), Status::NotOK);
+
+ auto x = g("twice");
+ ASSERT_EQ(x.ToStatus().GetCode(), Status::cOK);
+ auto y = g("");
+ ASSERT_EQ(y.ToStatus().GetCode(), Status::NotOK);
+}
+
+TEST(StatusOr, SharedPtr) {
+ struct A {
+ A(int *x) : x(x) { *x = 233; }
+ ~A() { *x = 0; }
+
+ int *x;
+ };
+
+ int val = 0;
+
+ {
+ StatusOr<std::shared_ptr<A>> x(new A(&val));
+
+ ASSERT_EQ(val, 233);
+ ASSERT_EQ(x->use_count(), 1);
+
+ {
+ StatusOr<std::shared_ptr<A>> y(*x);
+ ASSERT_EQ(val, 233);
+ ASSERT_EQ(x->use_count(), 2);
+ }
+
+ ASSERT_EQ(x->use_count(), 1);
+ }
+
+ ASSERT_EQ(val, 0);
+
+}
