From dcb8ae9be121cdae6f6d16757d2233d705df057d Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Fri, 3 Nov 2023 11:32:49 -0400 Subject: [PATCH] cpp-common/bt2: systematize the `const` method situation This patch attempts to systematize the `const` situation regarding methods (member functions and operators). The current issue is that, because those wrappers are expected to be passed and returned by copy (they only wrap a libbabeltrace2 pointer), one cannot rely on the `const` qualifier to ensure immutability at the library level. For example: void f(const bt2::ArrayValue val) { // Want to modify what `val` wraps? No problem: auto newVal = val; newVal += 23; } The signature of f() here doesn't guarantee to the caller that what `val` wraps is immutable. To achieve this, it needs to accept the `Const` version of the wrapper: void f(bt2::ConstArrayValue val) { // Now there's no way to modify what `val` wraps } Therefore, since using the `const` keyword is so fragile, this patch makes `const` (almost) all the methods of wrapper classes: those methods do not modify the wrapped pointer value, therefore they may be `const`. This makes the wrapping system have this correspondence: bt_xyz * bt2::Xyz bt_xyz * const const bt2::Xyz const bt_xyz * bt2::ConstXyz const bt_xyz * const const bt2::ConstXyz Because there were many overloads to return a `bt2::Const*` instance when the object is `const`, this patch removes a lot of code, with the added benefit of reducing code duplication. Signed-off-by: Philippe Proulx Change-Id: Ie633e7e9d67a8eb0138800ad8ae14c8d9bd0ca4c Reviewed-on: https://review.lttng.org/c/babeltrace/+/11228 Reviewed-by: Simon Marchi --- src/cpp-common/bt2/clock-class.hpp | 28 +- src/cpp-common/bt2/common-iterator.hpp | 4 +- src/cpp-common/bt2/field-class.hpp | 150 ++------ src/cpp-common/bt2/field.hpp | 159 ++------ src/cpp-common/bt2/integer-range-set.hpp | 2 +- src/cpp-common/bt2/message.hpp | 68 +--- src/cpp-common/bt2/trace-ir.hpp | 459 ++++++----------------- src/cpp-common/bt2/value.hpp | 130 +++---- 8 files changed, 241 insertions(+), 759 deletions(-) diff --git a/src/cpp-common/bt2/clock-class.hpp b/src/cpp-common/bt2/clock-class.hpp index 5e0e949d..398f8197 100644 --- a/src/cpp-common/bt2/clock-class.hpp +++ b/src/cpp-common/bt2/clock-class.hpp @@ -121,7 +121,7 @@ public: return *this; } - void frequency(const std::uint64_t frequency) noexcept + void frequency(const std::uint64_t frequency) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -133,7 +133,7 @@ public: return bt_clock_class_get_frequency(this->libObjPtr()); } - void offset(const ClockClassOffset& offset) noexcept + void offset(const ClockClassOffset& offset) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -149,7 +149,7 @@ public: return ClockClassOffset {seconds, cycles}; } - void precision(const std::uint64_t precision) noexcept + void precision(const std::uint64_t precision) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -161,7 +161,7 @@ public: return bt_clock_class_get_precision(this->libObjPtr()); } - void originIsUnixEpoch(const bool originIsUnixEpoch) noexcept + void originIsUnixEpoch(const bool originIsUnixEpoch) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -174,7 +174,7 @@ public: return static_cast(bt_clock_class_origin_is_unix_epoch(this->libObjPtr())); } - void name(const char * const name) + void name(const char * const name) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -185,7 +185,7 @@ public: } } - void name(const std::string& name) + void name(const std::string& name) const { this->name(name.data()); } @@ -201,7 +201,7 @@ public: return nonstd::nullopt; } - void description(const char * const description) + void description(const char * const description) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -212,7 +212,7 @@ public: } } - void description(const std::string& description) + void description(const std::string& description) const { this->description(description.data()); } @@ -228,7 +228,7 @@ public: return nonstd::nullopt; } - void uuid(const std::uint8_t * const uuid) noexcept + void uuid(const std::uint8_t * const uuid) const noexcept { bt_clock_class_set_uuid(this->libObjPtr(), uuid); } @@ -245,20 +245,14 @@ public: } template - void userAttributes(const CommonMapValue userAttrs) + void userAttributes(const CommonMapValue userAttrs) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); bt_clock_class_set_user_attributes(this->libObjPtr(), userAttrs.libObjPtr()); } - ConstMapValue userAttributes() const noexcept - { - return ConstMapValue {internal::CommonClockClassSpec::userAttributes( - this->libObjPtr())}; - } - - UserAttributes userAttributes() noexcept + UserAttributes userAttributes() const noexcept { return UserAttributes { internal::CommonClockClassSpec::userAttributes(this->libObjPtr())}; diff --git a/src/cpp-common/bt2/common-iterator.hpp b/src/cpp-common/bt2/common-iterator.hpp index e878b1b9..51b8d8e2 100644 --- a/src/cpp-common/bt2/common-iterator.hpp +++ b/src/cpp-common/bt2/common-iterator.hpp @@ -64,12 +64,12 @@ public: return !(*this == other); } - reference operator*() noexcept + reference operator*() const noexcept { return *_mCurrVal; } - pointer operator->() noexcept + pointer operator->() const noexcept { return &(*_mCurrVal); } diff --git a/src/cpp-common/bt2/field-class.hpp b/src/cpp-common/bt2/field-class.hpp index ec5df3c5..d382db1d 100644 --- a/src/cpp-common/bt2/field-class.hpp +++ b/src/cpp-common/bt2/field-class.hpp @@ -421,20 +421,14 @@ public: asVariantWithSignedIntegerSelector() const noexcept; template - void userAttributes(const CommonMapValue userAttrs) + void userAttributes(const CommonMapValue userAttrs) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); bt_field_class_set_user_attributes(this->libObjPtr(), userAttrs.libObjPtr()); } - ConstMapValue userAttributes() const noexcept - { - return ConstMapValue {internal::CommonFieldClassSpec::userAttributes( - this->libObjPtr())}; - } - - UserAttributes userAttributes() noexcept + UserAttributes userAttributes() const noexcept { return UserAttributes { internal::CommonFieldClassSpec::userAttributes(this->libObjPtr())}; @@ -579,7 +573,7 @@ public: return *this; } - void fieldValueRange(const std::uint64_t n) noexcept + void fieldValueRange(const std::uint64_t n) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -591,7 +585,7 @@ public: return bt_field_class_integer_get_field_value_range(this->libObjPtr()); } - void preferredDisplayBase(const DisplayBase base) noexcept + void preferredDisplayBase(const DisplayBase base) const noexcept { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -882,7 +876,7 @@ public: return (*this)[label.data()]; } - void addMapping(const char * const label, const typename Mapping::RangeSet ranges) + void addMapping(const char * const label, const typename Mapping::RangeSet ranges) const { const auto status = internal::CommonEnumerationFieldClassSpec::addMapping( this->libObjPtr(), label, ranges.libObjPtr()); @@ -892,7 +886,7 @@ public: } } - void addMapping(const std::string& label, const typename Mapping::RangeSet ranges) + void addMapping(const std::string& label, const typename Mapping::RangeSet ranges) const { this->addMapping(label.data(), ranges); } @@ -1038,20 +1032,14 @@ public: return bt_field_class_structure_member_get_name(this->libObjPtr()); } - ConstFieldClass fieldClass() const noexcept - { - return ConstFieldClass {internal::CommonStructureFieldClassMemberSpec< - const bt_field_class_structure_member>::fieldClass(this->libObjPtr())}; - } - - _FieldClass fieldClass() noexcept + _FieldClass fieldClass() const noexcept { return _FieldClass { internal::CommonStructureFieldClassMemberSpec::fieldClass(this->libObjPtr())}; } template - void userAttributes(const CommonMapValue userAttrs) + void userAttributes(const CommonMapValue userAttrs) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -1059,13 +1047,7 @@ public: userAttrs.libObjPtr()); } - ConstMapValue userAttributes() const noexcept - { - return ConstMapValue {internal::CommonStructureFieldClassMemberSpec< - const bt_field_class_structure_member>::userAttributes(this->libObjPtr())}; - } - - UserAttributes userAttributes() noexcept + UserAttributes userAttributes() const noexcept { return UserAttributes { internal::CommonStructureFieldClassMemberSpec::userAttributes( @@ -1169,7 +1151,7 @@ public: return *this; } - void appendMember(const char * const name, const FieldClass fc) + void appendMember(const char * const name, const FieldClass fc) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); @@ -1181,7 +1163,7 @@ public: } } - void appendMember(const std::string& name, const FieldClass fc) + void appendMember(const std::string& name, const FieldClass fc) const { this->appendMember(name.data(), fc); } @@ -1201,40 +1183,13 @@ public: return Iterator {*this, this->size()}; } - ConstStructureFieldClassMember operator[](const std::uint64_t index) const noexcept - { - return ConstStructureFieldClassMember { - internal::CommonStructureFieldClassSpec::memberByIndex( - this->libObjPtr(), index)}; - } - - Member operator[](const std::uint64_t index) noexcept + Member operator[](const std::uint64_t index) const noexcept { return Member {internal::CommonStructureFieldClassSpec::memberByIndex( this->libObjPtr(), index)}; } - nonstd::optional - operator[](const char * const name) const noexcept - { - const auto libObjPtr = - internal::CommonStructureFieldClassSpec::memberByName( - this->libObjPtr(), name); - - if (libObjPtr) { - return ConstStructureFieldClassMember {libObjPtr}; - } - - return nonstd::nullopt; - } - - nonstd::optional - operator[](const std::string& name) const noexcept - { - return (*this)[name.data()]; - } - - nonstd::optional operator[](const char * const name) noexcept + nonstd::optional operator[](const char * const name) const noexcept { const auto libObjPtr = internal::CommonStructureFieldClassSpec::memberByName(this->libObjPtr(), name); @@ -1246,7 +1201,7 @@ public: return nonstd::nullopt; } - nonstd::optional operator[](const std::string& name) noexcept + nonstd::optional operator[](const std::string& name) const noexcept { return (*this)[name.data()]; } @@ -1338,14 +1293,7 @@ public: return *this; } - ConstFieldClass elementFieldClass() const noexcept - { - return ConstFieldClass { - internal::CommonArrayFieldClassSpec::elementFieldClass( - this->libObjPtr())}; - } - - _FieldClass elementFieldClass() noexcept + _FieldClass elementFieldClass() const noexcept { return _FieldClass { internal::CommonArrayFieldClassSpec::elementFieldClass(this->libObjPtr())}; @@ -1573,14 +1521,7 @@ public: return *this; } - ConstFieldClass fieldClass() const noexcept - { - return ConstFieldClass { - internal::CommonOptionFieldClassSpec::fieldClass( - this->libObjPtr())}; - } - - _FieldClass fieldClass() noexcept + _FieldClass fieldClass() const noexcept { return _FieldClass { internal::CommonOptionFieldClassSpec::fieldClass(this->libObjPtr())}; @@ -1965,33 +1906,21 @@ public: return nonstd::nullopt; } - ConstFieldClass fieldClass() const noexcept - { - return ConstFieldClass {internal::CommonVariantFieldClassOptionSpec< - const bt_field_class_variant_option>::fieldClass(this->libObjPtr())}; - } - - _FieldClass fieldClass() noexcept + _FieldClass fieldClass() const noexcept { return _FieldClass { internal::CommonVariantFieldClassOptionSpec::fieldClass(this->libObjPtr())}; } template - void userAttributes(const CommonMapValue userAttrs) + void userAttributes(const CommonMapValue userAttrs) const { static_assert(!std::is_const::value, "`LibObjT` must NOT be `const`."); bt_field_class_variant_option_set_user_attributes(this->libObjPtr(), userAttrs.libObjPtr()); } - ConstMapValue userAttributes() const noexcept - { - return ConstMapValue {internal::CommonVariantFieldClassOptionSpec< - const bt_field_class_variant_option>::userAttributes(this->libObjPtr())}; - } - - UserAttributes userAttributes() noexcept + UserAttributes userAttributes() const noexcept { return UserAttributes {internal::CommonVariantFieldClassOptionSpec::userAttributes( this->libObjPtr())}; @@ -2230,40 +2159,13 @@ public: return Iterator {*this, this->size()}; } - ConstVariantFieldClassOption operator[](const std::uint64_t index) const noexcept - { - return ConstVariantFieldClassOption { - internal::CommonVariantFieldClassSpec::optionByIndex( - this->libObjPtr(), index)}; - } - - Option operator[](const std::uint64_t index) noexcept + Option operator[](const std::uint64_t index) const noexcept { return Option {internal::CommonVariantFieldClassSpec::optionByIndex( this->libObjPtr(), index)}; } - nonstd::optional - operator[](const char * const name) const noexcept - { - const auto libObjPtr = - internal::CommonVariantFieldClassSpec::optionByName( - this->libObjPtr(), name); - - if (libObjPtr) { - return ConstVariantFieldClassOption {libObjPtr}; - } - - return nonstd::nullopt; - } - - nonstd::optional - operator[](const std::string& name) const noexcept - { - return (*this)[name.data()]; - } - - nonstd::optional