Make chip::Optional be trivially destructible if the underlying type is. (project-chip#37017)

andy31415 · andreilitvin · web-flow · commit 448930e8bec1 · 2025-01-10T22:31:29.000Z
* Make chip::Optional be trivially destructible if the underlying type is.

Previous implementation always had a destructor, so it was never
trivially destructible.

* Remove extra char

* Remove unused variable

* Remove one more unused variable

---------

Co-authored-by: Andrei Litvin &lt;andreilitvin@google.com&gt;
diff --git a/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp b/examples/energy-management-app/energy-management-common/device-energy-management/src/DEMTestEventTriggers.cpp
@@ -36,7 +36,6 @@ constexpr uint16_t MAX_POWER_ADJUSTMENTS = 5;
 
 chip::app::Clusters::DeviceEnergyManagement::Structs::SlotStruct::Type sSlots[MAX_SLOTS];
 chip::app::Clusters::DeviceEnergyManagement::Structs::ForecastStruct::Type sForecastStruct;
-chip::app::DataModel::Nullable<chip::app::Clusters::DeviceEnergyManagement::Structs::ForecastStruct::Type> sForecast;
 
 chip::app::Clusters::DeviceEnergyManagement::Structs::PowerAdjustStruct::Type sPowerAdjustments[MAX_POWER_ADJUSTMENTS];
 chip::app::Clusters::DeviceEnergyManagement::Structs::PowerAdjustCapabilityStruct::Type sPowerAdjustCapabilityStruct;
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
@@ -309,7 +309,6 @@ CHIP_ERROR CloseValve(EndpointId ep)
 CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable<Percent> level, DataModel::Nullable<uint32_t> openDuration)
 {
     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))
diff --git a/src/lib/core/Optional.h b/src/lib/core/Optional.h
@@ -48,92 +48,92 @@ template <class T>
 class Optional
 {
 public:
-    constexpr Optional() : mHasValue(false) {}
-    constexpr Optional(NullOptionalType) : mHasValue(false) {}
+    constexpr Optional() {}
+    constexpr Optional(NullOptionalType) {}
 
-    ~Optional()
+    explicit Optional(const T & value)
     {
-        // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue
-        if (mHasValue)
-        {
-            mValue.mData.~T();
-        }
+        mValueHolder.mHasValue = true;
+        new (&mValueHolder.mValue.mData) T(value);
     }
 
-    explicit Optional(const T & value) : mHasValue(true) { new (&mValue.mData) T(value); }
-
     template <class... Args>
-    constexpr explicit Optional(InPlaceType, Args &&... args) : mHasValue(true)
+    constexpr explicit Optional(InPlaceType, Args &&... args)
     {
-        new (&mValue.mData) T(std::forward<Args>(args)...);
+        mValueHolder.mHasValue = true;
+        new (&mValueHolder.mValue.mData) T(std::forward<Args>(args)...);
     }
 
-    constexpr Optional(const Optional & other) : mHasValue(other.mHasValue)
+    constexpr Optional(const Optional & other)
     {
-        if (mHasValue)
+        mValueHolder.mHasValue = other.mValueHolder.mHasValue;
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(other.mValue.mData);
+            new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData);
         }
     }
 
     // Converts an Optional of an implicitly convertible type
     template <class U, std::enable_if_t<!std::is_same_v<T, U> && std::is_convertible_v<const U, T>, bool> = true>
-    constexpr Optional(const Optional<U> & other) : mHasValue(other.HasValue())
+    constexpr Optional(const Optional<U> & other)
     {
-        if (mHasValue)
+        mValueHolder.mHasValue = other.HasValue();
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(other.Value());
+            new (&mValueHolder.mValue.mData) T(other.Value());
         }
     }
 
     // Converts an Optional of a type that requires explicit conversion
     template <class U,
               std::enable_if_t<!std::is_same_v<T, U> && !std::is_convertible_v<const U, T> && std::is_constructible_v<T, const U &>,
                                bool> = true>
-    constexpr explicit Optional(const Optional<U> & other) : mHasValue(other.HasValue())
+    constexpr explicit Optional(const Optional<U> & other)
     {
-        if (mHasValue)
+        mValueHolder.mHasValue = other.HasValue();
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(other.Value());
+            new (&mValueHolder.mValue.mData) T(other.Value());
         }
     }
 
-    constexpr Optional(Optional && other) : mHasValue(other.mHasValue)
+    constexpr Optional(Optional && other)
     {
-        if (mHasValue)
+        mValueHolder.mHasValue = other.mValueHolder.mHasValue;
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(std::move(other.mValue.mData));
-            other.mValue.mData.~T();
-            other.mHasValue = false;
+            new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData));
+            other.mValueHolder.mValue.mData.~T();
+            other.mValueHolder.mHasValue = false;
         }
     }
 
     constexpr Optional & operator=(const Optional & other)
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = other.mHasValue;
-        if (mHasValue)
+        mValueHolder.mHasValue = other.mValueHolder.mHasValue;
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(other.mValue.mData);
+            new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData);
         }
         return *this;
     }
 
     constexpr Optional & operator=(Optional && other)
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = other.mHasValue;
-        if (mHasValue)
+        mValueHolder.mHasValue = other.mValueHolder.mHasValue;
+        if (mValueHolder.mHasValue)
         {
-            new (&mValue.mData) T(std::move(other.mValue.mData));
-            other.mValue.mData.~T();
-            other.mHasValue = false;
+            new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData));
+            other.mValueHolder.mValue.mData.~T();
+            other.mValueHolder.mHasValue = false;
         }
         return *this;
     }
@@ -142,24 +142,24 @@ class Optional
     template <class... Args>
     constexpr T & Emplace(Args &&... args)
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = true;
-        new (&mValue.mData) T(std::forward<Args>(args)...);
-        return mValue.mData;
+        mValueHolder.mHasValue = true;
+        new (&mValueHolder.mValue.mData) T(std::forward<Args>(args)...);
+        return mValueHolder.mValue.mData;
     }
 
     /** Make the optional contain a specific value */
     constexpr void SetValue(const T & value)
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = true;
-        new (&mValue.mData) T(value);
+        mValueHolder.mHasValue = true;
+        new (&mValueHolder.mValue.mData) T(value);
     }
 
     constexpr void SetValue(std::optional<T> & value)
@@ -177,48 +177,49 @@ class Optional
     /** Make the optional contain a specific value */
     constexpr void SetValue(T && value)
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = true;
-        new (&mValue.mData) T(std::move(value));
+        mValueHolder.mHasValue = true;
+        new (&mValueHolder.mValue.mData) T(std::move(value));
     }
 
     /** Invalidate the value inside the optional. Optional now has no value */
     constexpr void ClearValue()
     {
-        if (mHasValue)
+        if (mValueHolder.mHasValue)
         {
-            mValue.mData.~T();
+            mValueHolder.mValue.mData.~T();
         }
-        mHasValue = false;
+        mValueHolder.mHasValue = false;
     }
 
     /** Gets the current value of the optional. Valid IFF `HasValue`. */
     T & Value() &
     {
         VerifyOrDie(HasValue());
-        return mValue.mData;
+        return mValueHolder.mValue.mData;
     }
 
     /** Gets the current value of the optional. Valid IFF `HasValue`. */
     const T & Value() const &
     {
         VerifyOrDie(HasValue());
-        return mValue.mData;
+        return mValueHolder.mValue.mData;
     }
 
     /** Gets the current value of the optional if the optional has a value;
         otherwise returns the provided default value. */
     const T & ValueOr(const T & defaultValue) const { return HasValue() ? Value() : defaultValue; }
 
     /** Checks if the optional contains a value or not */
-    constexpr bool HasValue() const { return mHasValue; }
+    constexpr bool HasValue() const { return mValueHolder.mHasValue; }
 
     bool operator==(const Optional & other) const
     {
-        return (mHasValue == other.mHasValue) && (!other.mHasValue || (mValue.mData == other.mValue.mData));
+        return (mValueHolder.mHasValue == other.mValueHolder.mHasValue) &&
+            (!other.mValueHolder.mHasValue || (mValueHolder.mValue.mData == other.mValueHolder.mValue.mData));
     }
     bool operator!=(const Optional & other) const { return !(*this == other); }
     bool operator==(const T & other) const { return HasValue() && Value() == other; }
@@ -241,13 +242,47 @@ class Optional
     }
 
 private:
-    bool mHasValue;
-    union Value
+    // A container of bool + value (without constructor/destructor) when the underlying
+    // type has a trivial destructor
+    class TrivialDestructor
+    {
+    public:
+        bool mHasValue = false;
+        union Value
+        {
+            Value() {}
+            T mData;
+        } mValue;
+    };
+
+    // A container of bool + value that destroys the underlying type when mHasValue is true.
+    // To be used for non-trivial destructor classes.
+    class NonTrivialDestructor
+    {
+    public:
+        ~NonTrivialDestructor()
+        {
+            // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue
+            if (mHasValue)
+            {
+                mValue.mData.~T();
+            }
+        }
+
+        bool mHasValue = false;
+        union Value
+        {
+            Value() {}
+            ~Value() {}
+            T mData;
+        } mValue;
+    };
+
+    class ValueHolder : public std::conditional_t<std::is_trivially_destructible_v<T>, TrivialDestructor, NonTrivialDestructor>
     {
-        Value() {}
-        ~Value() {}
-        T mData;
-    } mValue;
+    };
+
+    ValueHolder mValueHolder;
 };
 
 template <class T>
diff --git a/src/lib/core/tests/TestOptional.cpp b/src/lib/core/tests/TestOptional.cpp
@@ -17,22 +17,18 @@
  *    limitations under the License.
  */
 
-/**
- *    @file
- *      This file implements a test for  CHIP core library reference counted object.
- *
- */
-
 #include <array>
 #include <cinttypes>
 #include <cstdint>
 #include <cstring>
+#include <string>
 
 #include <pw_unit_test/framework.h>
 
 #include <lib/core/Optional.h>
 #include <lib/core/StringBuilderAdapters.h>
 #include <lib/support/Span.h>
+#include <type_traits>
 
 using namespace chip;
 
@@ -76,6 +72,10 @@ struct CountMovable : public Count
 int Count::created;
 int Count::destroyed;
 
+// Optional is trivially destructible if the underlying type is trivially destructible
+static_assert(std::is_trivially_destructible_v<chip::Optional<unsigned>>);
+static_assert(!std::is_trivially_destructible_v<chip::Optional<std::string>>);
+
 TEST(TestOptional, TestBasic)
 {
     // Set up our test Count objects, which will mess with counts, before we reset the

Original file line number	Diff line number	Diff line change
`@@ -309,7 +309,6 @@ CHIP_ERROR CloseValve(EndpointId ep)`
`309`	`309`	`CHIP_ERROR SetValveLevel(EndpointId ep, DataModel::Nullable<Percent> level, DataModel::Nullable<uint32_t> openDuration)`
`310`	`310`	`{`
`311`	`311`	`Delegate * delegate = GetDelegate(ep);`
`312`		`- Optional<Status> status = Optional<Status>::Missing();`
`313`	`312`	`CHIP_ERROR attribute_error = CHIP_IM_GLOBAL_STATUS(UnsupportedAttribute);`
`314`	`313`
`315`	`314`	`if (HasFeature(ep, ValveConfigurationAndControl::Feature::kTimeSync))`
Original file line number	Diff line number	Diff line change
`@@ -48,92 +48,92 @@ template <class T>`
`48`	`48`	`class Optional`
`49`	`49`	`{`
`50`	`50`	`public:`
`51`		`- constexpr Optional() : mHasValue(false) {}`
`52`		`- constexpr Optional(NullOptionalType) : mHasValue(false) {}`
	`51`	`+ constexpr Optional() {}`
	`52`	`+ constexpr Optional(NullOptionalType) {}`
`53`	`53`
`54`		`- ~Optional()`
	`54`	`+ explicit Optional(const T & value)`
`55`	`55`	`{`
`56`		`- // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue`
`57`		`- if (mHasValue)`
`58`		`- {`
`59`		`- mValue.mData.~T();`
`60`		`- }`
	`56`	`+ mValueHolder.mHasValue = true;`
	`57`	`+ new (&mValueHolder.mValue.mData) T(value);`
`61`	`58`	`}`
`62`	`59`
`63`		`- explicit Optional(const T & value) : mHasValue(true) { new (&mValue.mData) T(value); }`
`64`		`-`
`65`	`60`	`template <class... Args>`
`66`		`- constexpr explicit Optional(InPlaceType, Args &&... args) : mHasValue(true)`
	`61`	`+ constexpr explicit Optional(InPlaceType, Args &&... args)`
`67`	`62`	`{`
`68`		`- new (&mValue.mData) T(std::forward<Args>(args)...);`
	`63`	`+ mValueHolder.mHasValue = true;`
	`64`	`+ new (&mValueHolder.mValue.mData) T(std::forward<Args>(args)...);`
`69`	`65`	`}`
`70`	`66`
`71`		`- constexpr Optional(const Optional & other) : mHasValue(other.mHasValue)`
	`67`	`+ constexpr Optional(const Optional & other)`
`72`	`68`	`{`
`73`		`- if (mHasValue)`
	`69`	`+ mValueHolder.mHasValue = other.mValueHolder.mHasValue;`
	`70`	`+ if (mValueHolder.mHasValue)`
`74`	`71`	`{`
`75`		`- new (&mValue.mData) T(other.mValue.mData);`
	`72`	`+ new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData);`
`76`	`73`	`}`
`77`	`74`	`}`
`78`	`75`
`79`	`76`	`// Converts an Optional of an implicitly convertible type`
`80`	`77`	`template <class U, std::enable_if_t<!std::is_same_v<T, U> && std::is_convertible_v<const U, T>, bool> = true>`
`81`		`- constexpr Optional(const Optional<U> & other) : mHasValue(other.HasValue())`
	`78`	`+ constexpr Optional(const Optional<U> & other)`
`82`	`79`	`{`
`83`		`- if (mHasValue)`
	`80`	`+ mValueHolder.mHasValue = other.HasValue();`
	`81`	`+ if (mValueHolder.mHasValue)`
`84`	`82`	`{`
`85`		`- new (&mValue.mData) T(other.Value());`
	`83`	`+ new (&mValueHolder.mValue.mData) T(other.Value());`
`86`	`84`	`}`
`87`	`85`	`}`
`88`	`86`
`89`	`87`	`// Converts an Optional of a type that requires explicit conversion`
`90`	`88`	`template <class U,`
`91`	`89`	`std::enable_if_t<!std::is_same_v<T, U> && !std::is_convertible_v<const U, T> && std::is_constructible_v<T, const U &>,`
`92`	`90`	`bool> = true>`
`93`		`- constexpr explicit Optional(const Optional<U> & other) : mHasValue(other.HasValue())`
	`91`	`+ constexpr explicit Optional(const Optional<U> & other)`
`94`	`92`	`{`
`95`		`- if (mHasValue)`
	`93`	`+ mValueHolder.mHasValue = other.HasValue();`
	`94`	`+ if (mValueHolder.mHasValue)`
`96`	`95`	`{`
`97`		`- new (&mValue.mData) T(other.Value());`
	`96`	`+ new (&mValueHolder.mValue.mData) T(other.Value());`
`98`	`97`	`}`
`99`	`98`	`}`
`100`	`99`
`101`		`- constexpr Optional(Optional && other) : mHasValue(other.mHasValue)`
	`100`	`+ constexpr Optional(Optional && other)`
`102`	`101`	`{`
`103`		`- if (mHasValue)`
	`102`	`+ mValueHolder.mHasValue = other.mValueHolder.mHasValue;`
	`103`	`+ if (mValueHolder.mHasValue)`
`104`	`104`	`{`
`105`		`- new (&mValue.mData) T(std::move(other.mValue.mData));`
`106`		`- other.mValue.mData.~T();`
`107`		`- other.mHasValue = false;`
	`105`	`+ new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData));`
	`106`	`+ other.mValueHolder.mValue.mData.~T();`
	`107`	`+ other.mValueHolder.mHasValue = false;`
`108`	`108`	`}`
`109`	`109`	`}`
`110`	`110`
`111`	`111`	`constexpr Optional & operator=(const Optional & other)`
`112`	`112`	`{`
`113`		`- if (mHasValue)`
	`113`	`+ if (mValueHolder.mHasValue)`
`114`	`114`	`{`
`115`		`- mValue.mData.~T();`
	`115`	`+ mValueHolder.mValue.mData.~T();`
`116`	`116`	`}`
`117`		`- mHasValue = other.mHasValue;`
`118`		`- if (mHasValue)`
	`117`	`+ mValueHolder.mHasValue = other.mValueHolder.mHasValue;`
	`118`	`+ if (mValueHolder.mHasValue)`
`119`	`119`	`{`
`120`		`- new (&mValue.mData) T(other.mValue.mData);`
	`120`	`+ new (&mValueHolder.mValue.mData) T(other.mValueHolder.mValue.mData);`
`121`	`121`	`}`
`122`	`122`	`return *this;`
`123`	`123`	`}`
`124`	`124`
`125`	`125`	`constexpr Optional & operator=(Optional && other)`
`126`	`126`	`{`
`127`		`- if (mHasValue)`
	`127`	`+ if (mValueHolder.mHasValue)`
`128`	`128`	`{`
`129`		`- mValue.mData.~T();`
	`129`	`+ mValueHolder.mValue.mData.~T();`
`130`	`130`	`}`
`131`		`- mHasValue = other.mHasValue;`
`132`		`- if (mHasValue)`
	`131`	`+ mValueHolder.mHasValue = other.mValueHolder.mHasValue;`
	`132`	`+ if (mValueHolder.mHasValue)`
`133`	`133`	`{`
`134`		`- new (&mValue.mData) T(std::move(other.mValue.mData));`
`135`		`- other.mValue.mData.~T();`
`136`		`- other.mHasValue = false;`
	`134`	`+ new (&mValueHolder.mValue.mData) T(std::move(other.mValueHolder.mValue.mData));`
	`135`	`+ other.mValueHolder.mValue.mData.~T();`
	`136`	`+ other.mValueHolder.mHasValue = false;`
`137`	`137`	`}`
`138`	`138`	`return *this;`
`139`	`139`	`}`
`@@ -142,24 +142,24 @@ class Optional`
`142`	`142`	`template <class... Args>`
`143`	`143`	`constexpr T & Emplace(Args &&... args)`
`144`	`144`	`{`
`145`		`- if (mHasValue)`
	`145`	`+ if (mValueHolder.mHasValue)`
`146`	`146`	`{`
`147`		`- mValue.mData.~T();`
	`147`	`+ mValueHolder.mValue.mData.~T();`
`148`	`148`	`}`
`149`		`- mHasValue = true;`
`150`		`- new (&mValue.mData) T(std::forward<Args>(args)...);`
`151`		`- return mValue.mData;`
	`149`	`+ mValueHolder.mHasValue = true;`
	`150`	`+ new (&mValueHolder.mValue.mData) T(std::forward<Args>(args)...);`
	`151`	`+ return mValueHolder.mValue.mData;`
`152`	`152`	`}`
`153`	`153`
`154`	`154`	`/** Make the optional contain a specific value */`
`155`	`155`	`constexpr void SetValue(const T & value)`
`156`	`156`	`{`
`157`		`- if (mHasValue)`
	`157`	`+ if (mValueHolder.mHasValue)`
`158`	`158`	`{`
`159`		`- mValue.mData.~T();`
	`159`	`+ mValueHolder.mValue.mData.~T();`
`160`	`160`	`}`
`161`		`- mHasValue = true;`
`162`		`- new (&mValue.mData) T(value);`
	`161`	`+ mValueHolder.mHasValue = true;`
	`162`	`+ new (&mValueHolder.mValue.mData) T(value);`
`163`	`163`	`}`
`164`	`164`
`165`	`165`	`constexpr void SetValue(std::optional<T> & value)`
`@@ -177,48 +177,49 @@ class Optional`
`177`	`177`	`/** Make the optional contain a specific value */`
`178`	`178`	`constexpr void SetValue(T && value)`
`179`	`179`	`{`
`180`		`- if (mHasValue)`
	`180`	`+ if (mValueHolder.mHasValue)`
`181`	`181`	`{`
`182`		`- mValue.mData.~T();`
	`182`	`+ mValueHolder.mValue.mData.~T();`
`183`	`183`	`}`
`184`		`- mHasValue = true;`
`185`		`- new (&mValue.mData) T(std::move(value));`
	`184`	`+ mValueHolder.mHasValue = true;`
	`185`	`+ new (&mValueHolder.mValue.mData) T(std::move(value));`
`186`	`186`	`}`
`187`	`187`
`188`	`188`	`/** Invalidate the value inside the optional. Optional now has no value */`
`189`	`189`	`constexpr void ClearValue()`
`190`	`190`	`{`
`191`		`- if (mHasValue)`
	`191`	`+ if (mValueHolder.mHasValue)`
`192`	`192`	`{`
`193`		`- mValue.mData.~T();`
	`193`	`+ mValueHolder.mValue.mData.~T();`
`194`	`194`	`}`
`195`		`- mHasValue = false;`
	`195`	`+ mValueHolder.mHasValue = false;`
`196`	`196`	`}`
`197`	`197`
`198`	`198`	/** Gets the current value of the optional. Valid IFF `HasValue`. */
`199`	`199`	`T & Value() &`
`200`	`200`	`{`
`201`	`201`	`VerifyOrDie(HasValue());`
`202`		`- return mValue.mData;`
	`202`	`+ return mValueHolder.mValue.mData;`
`203`	`203`	`}`
`204`	`204`
`205`	`205`	/** Gets the current value of the optional. Valid IFF `HasValue`. */
`206`	`206`	`const T & Value() const &`
`207`	`207`	`{`
`208`	`208`	`VerifyOrDie(HasValue());`
`209`		`- return mValue.mData;`
	`209`	`+ return mValueHolder.mValue.mData;`
`210`	`210`	`}`
`211`	`211`
`212`	`212`	`/** Gets the current value of the optional if the optional has a value;`
`213`	`213`	`otherwise returns the provided default value. */`
`214`	`214`	`const T & ValueOr(const T & defaultValue) const { return HasValue() ? Value() : defaultValue; }`
`215`	`215`
`216`	`216`	`/** Checks if the optional contains a value or not */`
`217`		`- constexpr bool HasValue() const { return mHasValue; }`
	`217`	`+ constexpr bool HasValue() const { return mValueHolder.mHasValue; }`
`218`	`218`
`219`	`219`	`bool operator==(const Optional & other) const`
`220`	`220`	`{`
`221`		`- return (mHasValue == other.mHasValue) && (!other.mHasValue \|\| (mValue.mData == other.mValue.mData));`
	`221`	`+ return (mValueHolder.mHasValue == other.mValueHolder.mHasValue) &&`
	`222`	`+ (!other.mValueHolder.mHasValue \|\| (mValueHolder.mValue.mData == other.mValueHolder.mValue.mData));`
`222`	`223`	`}`
`223`	`224`	`bool operator!=(const Optional & other) const { return !(*this == other); }`
`224`	`225`	`bool operator==(const T & other) const { return HasValue() && Value() == other; }`
`@@ -241,13 +242,47 @@ class Optional`
`241`	`242`	`}`
`242`	`243`
`243`	`244`	`private:`
`244`		`- bool mHasValue;`
`245`		`- union Value`
	`245`	`+ // A container of bool + value (without constructor/destructor) when the underlying`
	`246`	`+ // type has a trivial destructor`
	`247`	`+ class TrivialDestructor`
	`248`	`+ {`
	`249`	`+ public:`
	`250`	`+ bool mHasValue = false;`
	`251`	`+ union Value`
	`252`	`+ {`
	`253`	`+ Value() {}`
	`254`	`+ T mData;`
	`255`	`+ } mValue;`
	`256`	`+ };`
	`257`	`+`
	`258`	`+ // A container of bool + value that destroys the underlying type when mHasValue is true.`
	`259`	`+ // To be used for non-trivial destructor classes.`
	`260`	`+ class NonTrivialDestructor`
	`261`	`+ {`
	`262`	`+ public:`
	`263`	`+ ~NonTrivialDestructor()`
	`264`	`+ {`
	`265`	`+ // NOLINTNEXTLINE(clang-analyzer-core.uninitialized.Branch): mData is set when mHasValue`
	`266`	`+ if (mHasValue)`
	`267`	`+ {`
	`268`	`+ mValue.mData.~T();`
	`269`	`+ }`
	`270`	`+ }`
	`271`	`+`
	`272`	`+ bool mHasValue = false;`
	`273`	`+ union Value`
	`274`	`+ {`
	`275`	`+ Value() {}`
	`276`	`+ ~Value() {}`
	`277`	`+ T mData;`
	`278`	`+ } mValue;`
	`279`	`+ };`
	`280`	`+`
	`281`	`+ class ValueHolder : public std::conditional_t<std::is_trivially_destructible_v<T>, TrivialDestructor, NonTrivialDestructor>`
`246`	`282`	`{`
`247`		`- Value() {}`
`248`		`- ~Value() {}`
`249`		`- T mData;`
`250`		`- } mValue;`
	`283`	`+ };`
	`284`	`+`
	`285`	`+ ValueHolder mValueHolder;`
`251`	`286`	`};`
`252`	`287`
`253`	`288`	`template <class T>`