From b3f060ed119961106d34da4db9071653a37f5c53 Mon Sep 17 00:00:00 2001
From: Philippe Proulx <eeppeliteloop@gmail.com>
Date: Sat, 11 Nov 2023 11:46:36 -0500
Subject: [PATCH] cpp-common/bt2: use raw value proxy for scalar value classes

This patch adds the `bt2::RawValueProxy` class of which an instance
contains a wrapper object `_mObj` and:

* Its `=` operator calls `_mObj.value()` to assign a value.
* Its raw value type operator returns what `_mObj.value()` returns.

Then, in scalar value wrapper classes of `value.hpp`:

* Remove the `=` operator which assigns a raw value.
* Add a value() method to assign a raw value.
* Add a `*` operator to return a raw value proxy containing this.

The goal of this patch is to avoid using the `=` operator on some value
object to both wrap another library object (pseudo copy assignment
operator) and assign a raw value. Now, to assign a raw value, you need
to "dereference" the wrapper object:

    void f(const bt2::UnsignedIntegerValue val) {
        *val = 23;
        std::cout << *val << std::endl;
    }

There's also `bt2::RawStringValueProxy` which adds the `=` operator to
`bt2::RawValueProxy` to assign a `const std::string&`.
`bt2::CommonStringValue` uses it.

Additionally, remove all the `=` operators to assign a raw value on
`bt2::CommonValue` and create a "master" raw value proxy class
(`CommonValueRawValueProxy`) to have all the `=` operators and raw value
type operators:

    void f(const bt2::Value val) {
        // We know it's a boolean value
        *val = true;
    }

Signed-off-by: Philippe Proulx <eeppeliteloop@gmail.com>
Change-Id: Ia8559de36baba3021169b3d097ec83b914407f63
Reviewed-on: https://review.lttng.org/c/babeltrace/+/11362
Reviewed-by: Simon Marchi <simon.marchi@efficios.com>
---
 src/Makefile.am                        |   1 +
 src/cpp-common/bt2/raw-value-proxy.hpp |  57 +++++++
 src/cpp-common/bt2/value.hpp           | 213 ++++++++++++++++---------
 3 files changed, 200 insertions(+), 71 deletions(-)
 create mode 100644 src/cpp-common/bt2/raw-value-proxy.hpp

diff --git a/src/Makefile.am b/src/Makefile.am
index 4d6eeb69..ee539f8e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -27,6 +27,7 @@ noinst_HEADERS = \
 	cpp-common/bt2/logging.hpp \
 	cpp-common/bt2/message.hpp \
 	cpp-common/bt2/shared-object.hpp \
+	cpp-common/bt2/raw-value-proxy.hpp \
 	cpp-common/bt2/trace-ir.hpp \
 	cpp-common/bt2/type-traits.hpp \
 	cpp-common/bt2/value.hpp \
diff --git a/src/cpp-common/bt2/raw-value-proxy.hpp b/src/cpp-common/bt2/raw-value-proxy.hpp
new file mode 100644
index 00000000..2153856f
--- /dev/null
+++ b/src/cpp-common/bt2/raw-value-proxy.hpp
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2023 Philippe Proulx <pproulx@efficios.com>
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+#ifndef BABELTRACE_CPP_COMMON_BT2_RAW_VALUE_PROXY_HPP
+#define BABELTRACE_CPP_COMMON_BT2_RAW_VALUE_PROXY_HPP
+
+#include <string>
+
+namespace bt2 {
+
+template <typename ObjT>
+class RawValueProxy
+{
+private:
+    using _RawVal = typename ObjT::Value;
+
+public:
+    explicit RawValueProxy(const ObjT obj) : _mObj {obj}
+    {
+    }
+
+    RawValueProxy& operator=(const _RawVal& rawVal)
+    {
+        _mObj.value(rawVal);
+        return *this;
+    }
+
+    operator _RawVal() const noexcept
+    {
+        return _mObj.value();
+    }
+
+private:
+    ObjT _mObj;
+};
+
+template <typename ObjT>
+class RawStringValueProxy final : public RawValueProxy<ObjT>
+{
+public:
+    explicit RawStringValueProxy(const ObjT obj) : RawValueProxy<ObjT> {obj}
+    {
+    }
+
+    RawStringValueProxy& operator=(const std::string& rawVal)
+    {
+        RawValueProxy<ObjT>::operator=(rawVal.data());
+        return *this;
+    }
+};
+
+} /* namespace bt2 */
+
+#endif /* BABELTRACE_CPP_COMMON_BT2_RAW_VALUE_PROXY_HPP */
diff --git a/src/cpp-common/bt2/value.hpp b/src/cpp-common/bt2/value.hpp
index 003ddffd..fe5a434d 100644
--- a/src/cpp-common/bt2/value.hpp
+++ b/src/cpp-common/bt2/value.hpp
@@ -22,6 +22,7 @@
 #include "borrowed-object.hpp"
 #include "exc.hpp"
 #include "internal/utils.hpp"
+#include "raw-value-proxy.hpp"
 #include "shared-object.hpp"
 
 namespace bt2 {
@@ -81,6 +82,30 @@ enum class ValueType
     MAP = BT_VALUE_TYPE_MAP,
 };
 
+template <typename ValueObjT>
+class CommonValueRawValueProxy final
+{
+public:
+    explicit CommonValueRawValueProxy(const ValueObjT obj) : _mObj {obj}
+    {
+    }
+
+    CommonValueRawValueProxy& operator=(bool rawVal) noexcept;
+    CommonValueRawValueProxy& operator=(std::int64_t rawVal) noexcept;
+    CommonValueRawValueProxy& operator=(std::uint64_t rawVal) noexcept;
+    CommonValueRawValueProxy& operator=(double rawVal) noexcept;
+    CommonValueRawValueProxy& operator=(const char *rawVal);
+    CommonValueRawValueProxy& operator=(const std::string& rawVal);
+    operator bool() const noexcept;
+    operator std::int64_t() const noexcept;
+    operator std::uint64_t() const noexcept;
+    operator double() const noexcept;
+    operator const char *() const noexcept;
+
+private:
+    ValueObjT _mObj;
+};
+
 template <typename LibObjT>
 class CommonValue : public BorrowedObject<LibObjT>
 {
@@ -177,40 +202,9 @@ public:
         return !(*this == other);
     }
 
-    CommonValue<LibObjT> operator=(const bool rawVal) const noexcept
-    {
-        this->asBool() = rawVal;
-        return *this;
-    }
-
-    CommonValue<LibObjT> operator=(const std::uint64_t rawVal) const noexcept
-    {
-        this->asUnsignedInteger() = rawVal;
-        return *this;
-    }
-
-    CommonValue<LibObjT> operator=(const std::int64_t rawVal) const noexcept
-    {
-        this->asSignedInteger() = rawVal;
-        return *this;
-    }
-
-    CommonValue<LibObjT> operator=(const double rawVal) const noexcept
+    CommonValueRawValueProxy<CommonValue> operator*() const noexcept
     {
-        this->asReal() = rawVal;
-        return *this;
-    }
-
-    CommonValue<LibObjT> operator=(const char * const rawVal) const noexcept
-    {
-        this->asString() = rawVal;
-        return *this;
-    }
-
-    CommonValue<LibObjT> operator=(const std::string& rawVal) const noexcept
-    {
-        this->asString() = rawVal;
-        return *this;
+        return CommonValueRawValueProxy<CommonValue> {*this};
     }
 
     std::uint64_t arrayLength() const noexcept
@@ -300,6 +294,83 @@ protected:
 using Value = CommonValue<bt_value>;
 using ConstValue = CommonValue<const bt_value>;
 
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const bool rawVal) noexcept
+{
+    _mObj.asBool().value(rawVal);
+    return *this;
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const std::int64_t rawVal) noexcept
+{
+    _mObj.asSignedInteger().value(rawVal);
+    return *this;
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const std::uint64_t rawVal) noexcept
+{
+    _mObj.asUnsignedInteger().value(rawVal);
+    return *this;
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const double rawVal) noexcept
+{
+    _mObj.asReal().value(rawVal);
+    return *this;
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const char * const rawVal)
+{
+    _mObj.asString().value(rawVal);
+    return *this;
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>&
+CommonValueRawValueProxy<ValueObjT>::operator=(const std::string& rawVal)
+{
+    return *this = rawVal.data();
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>::operator bool() const noexcept
+{
+    return _mObj.asBool().value();
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>::operator std::int64_t() const noexcept
+{
+    return _mObj.asSignedInteger().value();
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>::operator std::uint64_t() const noexcept
+{
+    return _mObj.asUnsignedInteger().value();
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>::operator double() const noexcept
+{
+    return _mObj.asReal().value();
+}
+
+template <typename ValueObjT>
+CommonValueRawValueProxy<ValueObjT>::operator const char *() const noexcept
+{
+    return _mObj.asString().value();
+}
+
 namespace internal {
 
 struct ValueTypeDescr
@@ -420,12 +491,9 @@ public:
         return CommonBoolValue<const bt_value> {*this};
     }
 
-    CommonBoolValue<LibObjT> operator=(const Value rawVal) const noexcept
+    RawValueProxy<CommonBoolValue> operator*() const noexcept
     {
-        static_assert(!std::is_const<LibObjT>::value, "Not available with `bt2::ConstBoolValue`.");
-
-        bt_value_bool_set(this->libObjPtr(), static_cast<bt_bool>(rawVal));
-        return *this;
+        return RawValueProxy<CommonBoolValue> {*this};
     }
 
     Value value() const noexcept
@@ -433,9 +501,11 @@ public:
         return static_cast<Value>(bt_value_bool_get(this->libObjPtr()));
     }
 
-    operator Value() const noexcept
+    void value(const Value val) const noexcept
     {
-        return this->value();
+        static_assert(!std::is_const<LibObjT>::value, "Not available with `bt2::ConstBoolValue`.");
+
+        bt_value_bool_set(this->libObjPtr(), static_cast<bt_bool>(val));
     }
 
     Shared shared() const noexcept
@@ -511,13 +581,17 @@ public:
         return CommonUnsignedIntegerValue<const bt_value> {*this};
     }
 
-    CommonUnsignedIntegerValue<LibObjT> operator=(const Value rawVal) const noexcept
+    RawValueProxy<CommonUnsignedIntegerValue> operator*() const noexcept
+    {
+        return RawValueProxy<CommonUnsignedIntegerValue> {*this};
+    }
+
+    void value(const Value val) const noexcept
     {
         static_assert(!std::is_const<LibObjT>::value,
                       "Not available with `bt2::ConstUnsignedIntegerValue`.");
 
-        bt_value_integer_unsigned_set(this->libObjPtr(), rawVal);
-        return *this;
+        bt_value_integer_unsigned_set(this->libObjPtr(), val);
     }
 
     Value value() const noexcept
@@ -525,11 +599,6 @@ public:
         return bt_value_integer_unsigned_get(this->libObjPtr());
     }
 
-    operator Value() const noexcept
-    {
-        return this->value();
-    }
-
     Shared shared() const noexcept
     {
         return Shared::createWithRef(*this);
@@ -603,13 +672,17 @@ public:
         return CommonSignedIntegerValue<const bt_value> {*this};
     }
 
-    CommonSignedIntegerValue<LibObjT> operator=(const Value rawVal) const noexcept
+    RawValueProxy<CommonSignedIntegerValue> operator*() const noexcept
+    {
+        return RawValueProxy<CommonSignedIntegerValue> {*this};
+    }
+
+    void value(const Value val) const noexcept
     {
         static_assert(!std::is_const<LibObjT>::value,
                       "Not available with `bt2::ConstSignedIntegerValue`.");
 
-        bt_value_integer_signed_set(this->libObjPtr(), rawVal);
-        return *this;
+        bt_value_integer_signed_set(this->libObjPtr(), val);
     }
 
     Value value() const noexcept
@@ -617,11 +690,6 @@ public:
         return bt_value_integer_signed_get(this->libObjPtr());
     }
 
-    operator Value() const noexcept
-    {
-        return this->value();
-    }
-
     Shared shared() const noexcept
     {
         return Shared::createWithRef(*this);
@@ -692,12 +760,16 @@ public:
         return CommonRealValue<const bt_value> {*this};
     }
 
-    CommonRealValue<LibObjT> operator=(const Value rawVal) const noexcept
+    RawValueProxy<CommonRealValue> operator*() const noexcept
+    {
+        return RawValueProxy<CommonRealValue> {*this};
+    }
+
+    void value(const Value val) const noexcept
     {
         static_assert(!std::is_const<LibObjT>::value, "Not available with `bt2::ConstRealValue`.");
 
-        bt_value_real_set(this->libObjPtr(), rawVal);
-        return *this;
+        bt_value_real_set(this->libObjPtr(), val);
     }
 
     Value value() const noexcept
@@ -705,11 +777,6 @@ public:
         return bt_value_real_get(this->libObjPtr());
     }
 
-    operator Value() const noexcept
-    {
-        return this->value();
-    }
-
     Shared shared() const noexcept
     {
         return Shared::createWithRef(*this);
@@ -748,6 +815,7 @@ private:
 
 public:
     using Shared = SharedValue<CommonStringValue<LibObjT>, LibObjT>;
+    using Value = const char *;
 
     explicit CommonStringValue(const _LibObjPtr libObjPtr) noexcept : _ThisCommonValue {libObjPtr}
     {
@@ -784,26 +852,29 @@ public:
         return CommonStringValue<const bt_value> {*this};
     }
 
-    CommonStringValue<LibObjT> operator=(const char * const rawVal) const
+    RawStringValueProxy<CommonStringValue> operator*() const noexcept
+    {
+        return RawStringValueProxy<CommonStringValue> {*this};
+    }
+
+    void value(const Value val) const
     {
         static_assert(!std::is_const<LibObjT>::value,
                       "Not available with `bt2::ConstStringValue`.");
 
-        const auto status = bt_value_string_set(this->libObjPtr(), rawVal);
+        const auto status = bt_value_string_set(this->libObjPtr(), val);
 
         if (status == BT_VALUE_STRING_SET_STATUS_MEMORY_ERROR) {
             throw MemoryError {};
         }
-
-        return *this;
     }
 
-    CommonStringValue<LibObjT> operator=(const std::string& rawVal) const noexcept
+    void value(const std::string& val) const
     {
-        return *this = rawVal.data();
+        this->value(val.data());
     }
 
-    bpstd::string_view value() const noexcept
+    const char *value() const noexcept
     {
         return bt_value_string_get(this->libObjPtr());
     }
-- 
2.34.1