Use std::optional for chip::Nullable (#33080)
* make nullable depend on std::optional instead of chip::optional
* Fix equality operator
* Fix inequality operator as well
* Fix test: previous code was enforcing chip::optional behavior on in-place construction
* Restyle
* Fix some compile errors ... std::optional now detects unused variables
* Restyle
* Remove unused variable
* Restyle
* Review updates
* Review fix
* Typo fixes
* Ugly forward of Value and ValueOr .... however at least hopefully it is complete
* Do not expose value/value_or at this time and keep Value/ValueOr as the public API for nullable
* Fix copy and paste typo
* Make more changes to make things compile and work like the old nullable
* Use value again instead of operator** ... I was looking at the wrong compiler errors
* Undo unrelated change
* Make clang-tidy be abel to compile ... apparently it really does not like using has_value in equality operators
* Restyle
* Another update to make clang-tidy happy for the other equality operator
* Update src/app/data-model/Nullable.h
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
* Undo odd whitespace change
---------
Co-authored-by: Andrei Litvin <andreilitvin@google.com>
Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
diff --git a/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp b/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
index 82dd454..02486be 100644
--- a/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
+++ b/src/app/clusters/valve-configuration-and-control-server/valve-configuration-and-control-server.cpp
@@ -277,8 +277,7 @@
CHIP_ERROR CloseValve(EndpointId ep)
{
- Delegate * delegate = GetDelegate(ep);
- DataModel::Nullable<uint32_t> rDuration;
+ Delegate * delegate = GetDelegate(ep);
CHIP_ERROR attribute_error = CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
VerifyOrReturnError(Status::Success == TargetState::Set(ep, ValveConfigurationAndControl::ValveStateEnum::kClosed),
@@ -309,10 +308,8 @@
CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable<Percent> level, DataModel::Nullable<uint32_t> openDuration)
{
- Delegate * delegate = GetDelegate(ep);
- Optional<Status> status = Optional<Status>::Missing();
- DataModel::Nullable<Percent> openLevel;
- DataModel::Nullable<uint64_t> autoCloseTime;
+ Delegate * delegate = GetDelegate(ep);
+ Optional<Status> status = Optional<Status>::Missing();
CHIP_ERROR attribute_error = CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);
if (HasFeature(ep, ValveConfigurationAndControl::Feature::kTimeSync))
@@ -328,6 +325,7 @@
VerifyOrReturnError(UnixEpochToChipEpochMicros(utcTime.count(), chipEpochTime), CHIP_ERROR_INVALID_TIME);
uint64_t time = openDuration.Value() * chip::kMicrosecondsPerSecond;
+ DataModel::Nullable<uint64_t> autoCloseTime;
autoCloseTime.SetNonNull(chipEpochTime + time);
VerifyOrReturnError(Status::Success == AutoCloseTime::Set(ep, autoCloseTime), attribute_error);
}
diff --git a/src/app/data-model/Nullable.h b/src/app/data-model/Nullable.h
index cea3dea..f56e67c 100644
--- a/src/app/data-model/Nullable.h
+++ b/src/app/data-model/Nullable.h
@@ -19,9 +19,9 @@
#pragma once
#include <app/util/attribute-storage-null-handling.h>
-#include <lib/core/Optional.h>
-
+#include <optional>
#include <type_traits>
+#include <utility>
namespace chip {
namespace app {
@@ -37,31 +37,40 @@
* things.
*/
template <typename T>
-struct Nullable : protected Optional<T>
+struct Nullable : protected std::optional<T>
{
+
//
// The following 'using' statement is needed to make visible
// all constructors of the base class within this derived class.
//
- using Optional<T>::Optional;
+ using std::optional<T>::optional;
+ using std::optional<T>::operator*;
+ using std::optional<T>::operator->;
- // Pull in APIs that make sense on Nullable with the same names as on
- // Optional.
- using Optional<T>::Value;
- using Optional<T>::ValueOr;
+ Nullable(NullOptionalType) : std::optional<T>(std::nullopt) {}
// Some consumers need an easy way to determine our underlying type.
using UnderlyingType = T;
- constexpr void SetNull() { Optional<T>::ClearValue(); }
- constexpr bool IsNull() const { return !Optional<T>::HasValue(); }
+ constexpr void SetNull() { std::optional<T>::reset(); }
+ constexpr bool IsNull() const { return !std::optional<T>::has_value(); }
template <class... Args>
constexpr T & SetNonNull(Args &&... args)
{
- return Optional<T>::Emplace(std::forward<Args>(args)...);
+ return std::optional<T>::emplace(std::forward<Args>(args)...);
}
+ template <typename... Args>
+ constexpr auto ValueOr(Args &&... args) const
+ {
+ return std::optional<T>::value_or(std::forward<Args>(args)...);
+ }
+
+ inline constexpr const T & Value() const { return std::optional<T>::value(); }
+ inline T & Value() { return std::optional<T>::value(); }
+
// For integer types, being nullable involves a range restriction.
template <
typename U = std::decay_t<T>,
@@ -96,22 +105,26 @@
// The only fabric-scoped objects in the spec are commands, events and structs inside lists, and none of those can be nullable.
static constexpr bool kIsFabricScoped = false;
- bool operator==(const Nullable & other) const { return Optional<T>::operator==(other); }
- bool operator!=(const Nullable & other) const { return Optional<T>::operator!=(other); }
- bool operator==(const T & other) const { return Optional<T>::operator==(other); }
- bool operator!=(const T & other) const { return Optional<T>::operator!=(other); }
+ inline bool operator==(const T & other) const { return static_cast<const std::optional<T> &>(*this) == other; }
+ inline bool operator!=(const T & other) const { return !(*this == other); }
+
+ inline bool operator==(const Nullable<T> & other) const
+ {
+ return static_cast<const std::optional<T> &>(*this) == static_cast<const std::optional<T> &>(other);
+ }
+ inline bool operator!=(const Nullable<T> & other) const { return !(*this == other); }
};
template <class T>
constexpr Nullable<std::decay_t<T>> MakeNullable(T && value)
{
- return Nullable<std::decay_t<T>>(InPlace, std::forward<T>(value));
+ return Nullable<std::decay_t<T>>(std::in_place, std::forward<T>(value));
}
template <class T, class... Args>
constexpr Nullable<T> MakeNullable(Args &&... args)
{
- return Nullable<T>(InPlace, std::forward<Args>(args)...);
+ return Nullable<T>(std::in_place, std::forward<Args>(args)...);
}
} // namespace DataModel
diff --git a/src/app/tests/TestNullable.cpp b/src/app/tests/TestNullable.cpp
index c1290c1..6600373 100644
--- a/src/app/tests/TestNullable.cpp
+++ b/src/app/tests/TestNullable.cpp
@@ -45,6 +45,7 @@
CtorDtorCounter & operator=(CtorDtorCounter &&) = default;
bool operator==(const CtorDtorCounter & o) const { return m == o.m; }
+ bool operator!=(const CtorDtorCounter & o) const { return m != o.m; }
int m;
@@ -68,6 +69,9 @@
MovableCtorDtorCounter(MovableCtorDtorCounter && o) = default;
MovableCtorDtorCounter & operator=(MovableCtorDtorCounter &&) = default;
+
+ using CtorDtorCounter::operator==;
+ using CtorDtorCounter::operator!=;
};
int CtorDtorCounter::created = 0;
@@ -165,24 +169,26 @@
CtorDtorCounter::ResetCounter();
{
- auto testSrc = MakeNullable<MovableCtorDtorCounter>(400);
- Nullable<MovableCtorDtorCounter> testDst(std::move(testSrc));
- NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 1);
+ auto testSrc = MakeNullable<MovableCtorDtorCounter>(400); // construct
+ Nullable<MovableCtorDtorCounter> testDst(std::move(testSrc)); // move construct
+ NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 0);
NL_TEST_ASSERT(inSuite, !testDst.IsNull() && testDst.Value().m == 400);
+ // destroy both testsSrc and testDst
}
NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 2);
+ CtorDtorCounter::ResetCounter();
{
- Nullable<MovableCtorDtorCounter> testDst;
- NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 2);
+ Nullable<MovableCtorDtorCounter> testDst; // no object construction
+ NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 0 && CtorDtorCounter::destroyed == 0);
NL_TEST_ASSERT(inSuite, !!testDst.IsNull());
- auto testSrc = MakeNullable<MovableCtorDtorCounter>(401);
- testDst = std::move(testSrc);
- NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 4 && CtorDtorCounter::destroyed == 3);
+ auto testSrc = MakeNullable<MovableCtorDtorCounter>(401); // construct object
+ testDst = std::move(testSrc); // construct a copy
+ NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 0);
NL_TEST_ASSERT(inSuite, !testDst.IsNull() && testDst.Value().m == 401);
}
- NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 4 && CtorDtorCounter::destroyed == 4);
+ NL_TEST_ASSERT(inSuite, CtorDtorCounter::created == 2 && CtorDtorCounter::destroyed == 2);
}
static void TestUpdate(nlTestSuite * inSuite, void * inContext)
