Add std::source_location to ChipError for C++20 builds (#32935)

* Add std::source_location to ChipError for C++20 builds * Fix errors due to CHIP_ERROR default constructor no longer being trivial * Restyled by clang-format * A few more fixes * Replace semicolon with comma * Add enum->int cast * Add tests and simplify constructor * Convert to gtest * Restyled by clang-format * Restyled by gn --------- Co-authored-by: Restyled.io <[email protected]>
project-chip · Apr 26, 2024 · 6722693 · 6722693
1 parent 1174bdc
commit 6722693
Show file tree

Hide file tree

Showing 14 changed files with 200 additions and 127 deletions.
diff --git a/src/app/FailSafeContext.cpp b/src/app/FailSafeContext.cpp
@@ -85,12 +85,11 @@ void FailSafeContext::ScheduleFailSafeCleanup(FabricIndex fabricIndex, bool addN
 
     SetFailSafeArmed(false);
 
-    ChipDeviceEvent event;
-    event.Type                                                = DeviceEventType::kFailSafeTimerExpired;
-    event.FailSafeTimerExpired.fabricIndex                    = fabricIndex;
-    event.FailSafeTimerExpired.addNocCommandHasBeenInvoked    = addNocCommandInvoked;
-    event.FailSafeTimerExpired.updateNocCommandHasBeenInvoked = updateNocCommandInvoked;
-    CHIP_ERROR status                                         = PlatformMgr().PostEvent(&event);
+    ChipDeviceEvent event{ .Type                 = DeviceEventType::kFailSafeTimerExpired,
+                           .FailSafeTimerExpired = { .fabricIndex                    = fabricIndex,
+                                                     .addNocCommandHasBeenInvoked    = addNocCommandInvoked,
+                                                     .updateNocCommandHasBeenInvoked = updateNocCommandInvoked } };
+    CHIP_ERROR status = PlatformMgr().PostEvent(&event);
 
     if (status != CHIP_NO_ERROR)
     {

diff --git a/src/app/clusters/bindings/bindings.cpp b/src/app/clusters/bindings/bindings.cpp
@@ -259,9 +259,8 @@ CHIP_ERROR BindingTableAccess::WriteBindingTable(const ConcreteDataAttributePath
 
 CHIP_ERROR BindingTableAccess::NotifyBindingsChanged()
 {
-    DeviceLayer::ChipDeviceEvent event;
-    event.Type                        = DeviceLayer::DeviceEventType::kBindingsChangedViaCluster;
-    event.BindingsChanged.fabricIndex = mAccessingFabricIndex;
+    DeviceLayer::ChipDeviceEvent event{ .Type            = DeviceLayer::DeviceEventType::kBindingsChangedViaCluster,
+                                        .BindingsChanged = { .fabricIndex = mAccessingFabricIndex } };
     return chip::DeviceLayer::PlatformMgr().PostEvent(&event);
 }
 

diff --git a/src/include/platform/CHIPDeviceEvent.h b/src/include/platform/CHIPDeviceEvent.h
@@ -530,7 +530,6 @@ struct ChipDeviceEvent final
         } OtaStateChanged;
     };
 
-    void Clear() { memset(this, 0, sizeof(*this)); }
     bool IsPublic() const { return DeviceEventType::IsPublic(Type); }
     bool IsInternal() const { return DeviceEventType::IsInternal(Type); }
     bool IsPlatformSpecific() const { return DeviceEventType::IsPlatformSpecific(Type); }

diff --git a/src/include/platform/internal/GenericConnectivityManagerImpl_Thread.ipp b/src/include/platform/internal/GenericConnectivityManagerImpl_Thread.ipp
@@ -70,11 +70,10 @@ void GenericConnectivityManagerImpl_Thread<ImplClass>::UpdateServiceConnectivity
         mFlags.Set(Flags::kHaveServiceConnectivity, haveServiceConnectivity);
 
         {
-            ChipDeviceEvent event;
-            event.Clear();
-            event.Type = DeviceEventType::kServiceConnectivityChange;
-            event.ServiceConnectivityChange.ViaThread.Result =
-                (haveServiceConnectivity) ? kConnectivity_Established : kConnectivity_Lost;
+            ChipDeviceEvent event{ .Type                      = DeviceEventType::kServiceConnectivityChange,
+                                   .ServiceConnectivityChange = { .ViaThread = { .Result = (haveServiceConnectivity)
+                                                                                     ? kConnectivity_Established
+                                                                                     : kConnectivity_Lost } } };
             event.ServiceConnectivityChange.Overall.Result = event.ServiceConnectivityChange.ViaThread.Result;
             CHIP_ERROR status                              = PlatformMgr().PostEvent(&event);
             if (status != CHIP_NO_ERROR)

diff --git a/src/lib/core/CHIPError.h b/src/lib/core/CHIPError.h
@@ -35,6 +35,10 @@
 #include <limits>
 #include <type_traits>
 
+#if __cplusplus >= 202002L
+#include <source_location>
+#endif // __cplusplus >= 202002L
+
 namespace chip {
 
 /**
@@ -112,9 +116,13 @@ class ChipError
 
     // Helper for declaring constructors without too much repetition.
 #if CHIP_CONFIG_ERROR_SOURCE
-#define CHIP_INITIALIZE_ERROR_SOURCE(f, l) , mFile((f)), mLine((l))
-#else // CHIP_CONFIG_ERROR_SOURCE
-#define CHIP_INITIALIZE_ERROR_SOURCE(f, l)
+#if __cplusplus >= 202002L
+#define CHIP_INITIALIZE_ERROR_SOURCE(f, l, loc) , mFile((f)), mLine((l)), mSourceLocation((loc))
+#else
+#define CHIP_INITIALIZE_ERROR_SOURCE(f, l, loc) , mFile((f)), mLine((l))
+#endif // __cplusplus >= 202002L
+#else  // CHIP_CONFIG_ERROR_SOURCE
+#define CHIP_INITIALIZE_ERROR_SOURCE(f, l, loc)
 #endif // CHIP_CONFIG_ERROR_SOURCE
 
     /**
@@ -123,23 +131,35 @@ class ChipError
      * @note
      *  The result is valid only if CanEncapsulate() is true.
      */
-    constexpr ChipError(Range range, ValueType value) :
-        mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(nullptr, 0)
+    constexpr ChipError(Range range, ValueType value) : ChipError(range, value, /*file=*/nullptr, /*line=*/0) {}
+#if __cplusplus >= 202002L
+    constexpr ChipError(Range range, ValueType value, const char * file, unsigned int line,
+                        std::source_location location = std::source_location::current()) :
+        mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
     {}
+#else
     constexpr ChipError(Range range, ValueType value, const char * file, unsigned int line) :
-        mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(file, line)
+        mError(MakeInteger(range, (value & MakeMask(0, kValueLength)))) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
+#endif // __cplusplus >= 202002L
 
     /**
      * Construct a CHIP_ERROR for SdkPart @a part with @a code.
      *
      * @note
      *  The macro version CHIP_SDK_ERROR checks that the numeric value is constant and well-formed.
      */
-    constexpr ChipError(SdkPart part, uint8_t code) : mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(nullptr, 0) {}
+    constexpr ChipError(SdkPart part, uint8_t code) : ChipError(part, code, /*file=*/nullptr, /*line=*/0) {}
+#if __cplusplus >= 202002L
+    constexpr ChipError(SdkPart part, uint8_t code, const char * file, unsigned int line,
+                        std::source_location location = std::source_location::current()) :
+        mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
+    {}
+#else
     constexpr ChipError(SdkPart part, uint8_t code, const char * file, unsigned int line) :
-        mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(file, line)
+        mError(MakeInteger(part, code)) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
+#endif // __cplusplus >= 202002L
 
     /**
      * Construct a CHIP_ERROR constant for SdkPart @a part with @a code at the current source line.
@@ -159,10 +179,17 @@ class ChipError
      * @note
      *  This is intended to be used only in foreign function interfaces.
      */
-    explicit constexpr ChipError(StorageType error) : mError(error) CHIP_INITIALIZE_ERROR_SOURCE(nullptr, 0) {}
+    explicit constexpr ChipError(StorageType error) : ChipError(error, /*file=*/nullptr, /*line=*/0) {}
+#if __cplusplus >= 202002L
+    explicit constexpr ChipError(StorageType error, const char * file, unsigned int line,
+                                 std::source_location location = std::source_location::current()) :
+        mError(error) CHIP_INITIALIZE_ERROR_SOURCE(file, line, location)
+    {}
+#else
     explicit constexpr ChipError(StorageType error, const char * file, unsigned int line) :
-        mError(error) CHIP_INITIALIZE_ERROR_SOURCE(file, line)
+        mError(error) CHIP_INITIALIZE_ERROR_SOURCE(file, line, /*loc=*/nullptr)
     {}
+#endif // __cplusplus >= 202002L
 
 #undef CHIP_INITIALIZE_ERROR_SOURCE
 
@@ -299,6 +326,12 @@ class ChipError
      */
     unsigned int GetLine() const { return mLine; }
 
+#if __cplusplus >= 202002L
+    /**
+     * Get the source_location of the point where the error occurred.
+     */
+    const std::source_location & GetSourceLocation() { return mSourceLocation; }
+#endif // __cplusplus >= 202002L
 #endif // CHIP_CONFIG_ERROR_SOURCE
 
 private:
@@ -365,6 +398,9 @@ class ChipError
 #if CHIP_CONFIG_ERROR_SOURCE
     const char * mFile;
     unsigned int mLine;
+#if __cplusplus >= 202002L
+    std::source_location mSourceLocation;
+#endif // __cplusplus >= 202002L
 #endif // CHIP_CONFIG_ERROR_SOURCE
 
 public:

diff --git a/src/lib/core/tests/BUILD.gn b/src/lib/core/tests/BUILD.gn
@@ -24,6 +24,7 @@ chip_test_suite("tests") {
   test_sources = [
     "TestCATValues.cpp",
     "TestCHIPCallback.cpp",
+    "TestCHIPError.cpp",
     "TestCHIPErrorStr.cpp",
     "TestOTAImageHeader.cpp",
     "TestOptional.cpp",

diff --git a/src/lib/core/tests/TestCHIPError.cpp b/src/lib/core/tests/TestCHIPError.cpp
@@ -0,0 +1,72 @@
+/*
+ *
+ *    Copyright (c) 2024 Project CHIP Authors
+ *    All rights reserved.
+ *
+ *    Licensed 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 <string>
+
+#include <lib/core/CHIPError.h>
+
+#include <gtest/gtest.h>
+
+namespace chip {
+namespace {
+
+TEST(ChipErrorTest, RangeConstructor)
+{
+    ChipError error(ChipError::Range::kSDK, /*value=*/1, __FILE__, __LINE__);
+#if CHIP_CONFIG_ERROR_SOURCE
+    EXPECT_EQ(error.GetFile(), __FILE__);
+    EXPECT_EQ(error.GetLine(), 30u);
+#if __cplusplus >= 202002L
+    std::source_location location = error.GetSourceLocation();
+    EXPECT_EQ(location.line(), 30u);
+    EXPECT_EQ(location.file_name(), __FILE__);
+#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE
+}
+
+TEST(ChipErrorTest, SdkPartConstructor)
+{
+    ChipError error(ChipError::SdkPart::kCore, /*code=*/1, __FILE__, __LINE__);
+#if CHIP_CONFIG_ERROR_SOURCE
+    EXPECT_EQ(error.GetFile(), __FILE__);
+    EXPECT_EQ(error.GetLine(), 44u);
+#if __cplusplus >= 202002L
+    std::source_location location = error.GetSourceLocation();
+    EXPECT_EQ(location.line(), 44u);
+    EXPECT_EQ(location.file_name(), __FILE__);
+#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE
+}
+
+TEST(ChipErrorTest, StorageTypeConstructor)
+{
+    ChipError error(/*error=*/1, __FILE__, __LINE__);
+    EXPECT_EQ(error.AsInteger(), 1u);
+#if CHIP_CONFIG_ERROR_SOURCE
+    EXPECT_EQ(error.GetFile(), __FILE__);
+    EXPECT_EQ(error.GetLine(), 58u);
+#if __cplusplus >= 202002L
+    std::source_location location = error.GetSourceLocation();
+    EXPECT_EQ(location.line(), 58u);
+    EXPECT_EQ(location.file_name(), __FILE__);
+#endif // __cplusplus >= 202002L
+#endif // CHIP_CONFIG_ERROR_SOURCE
+}
+
+} // namespace
+} // namespace chip
diff --git a/src/platform/DeviceControlServer.cpp b/src/platform/DeviceControlServer.cpp
@@ -36,11 +36,11 @@ DeviceControlServer & DeviceControlServer::DeviceControlSvr()
 
 CHIP_ERROR DeviceControlServer::PostCommissioningCompleteEvent(NodeId peerNodeId, FabricIndex accessingFabricIndex)
 {
-    ChipDeviceEvent event;
+    ChipDeviceEvent event{
 
-    event.Type                              = DeviceEventType::kCommissioningComplete;
-    event.CommissioningComplete.nodeId      = peerNodeId;
-    event.CommissioningComplete.fabricIndex = accessingFabricIndex;
+        .Type                  = DeviceEventType::kCommissioningComplete,
+        .CommissioningComplete = { .nodeId = peerNodeId, .fabricIndex = accessingFabricIndex }
+    };
 
     return PlatformMgr().PostEvent(&event);
 }
@@ -66,31 +66,27 @@ CHIP_ERROR DeviceControlServer::SetRegulatoryConfig(uint8_t location, const Char
 
 CHIP_ERROR DeviceControlServer::PostConnectedToOperationalNetworkEvent(ByteSpan networkID)
 {
-    ChipDeviceEvent event;
-    event.Type = DeviceEventType::kOperationalNetworkEnabled;
-    // TODO(cecille): This should be some way to specify thread or wifi.
-    event.OperationalNetwork.network = 0;
+    ChipDeviceEvent event{ .Type = DeviceEventType::kOperationalNetworkEnabled,
+                           // TODO(cecille): This should be some way to specify thread or wifi.
+                           .OperationalNetwork = { .network = 0 } };
     return PlatformMgr().PostEvent(&event);
 }
 
 CHIP_ERROR DeviceControlServer::PostCloseAllBLEConnectionsToOperationalNetworkEvent()
 {
-    ChipDeviceEvent event;
-    event.Type = DeviceEventType::kCloseAllBleConnections;
+    ChipDeviceEvent event{ .Type = DeviceEventType::kCloseAllBleConnections };
     return PlatformMgr().PostEvent(&event);
 }
 
 CHIP_ERROR DeviceControlServer::PostWiFiDeviceAvailableNetworkEvent()
 {
-    ChipDeviceEvent event;
-    event.Type = DeviceEventType::kWiFiDeviceAvailable;
+    ChipDeviceEvent event{ .Type = DeviceEventType::kWiFiDeviceAvailable };
     return PlatformMgr().PostEvent(&event);
 }
 
 CHIP_ERROR DeviceControlServer::PostOperationalNetworkStartedEvent()
 {
-    ChipDeviceEvent event;
-    event.Type = DeviceEventType::kOperationalNetworkStarted;
+    ChipDeviceEvent event{ .Type = DeviceEventType::kOperationalNetworkStarted };
     return PlatformMgr().PostEvent(&event);
 }