Try to reduce the codesize needed by EncodeListItem. (#37076)

We shouldn't need separate specializations of EncodeListItem for T, T&, const T, and const T&. Try to collapse those all down to "const T&".
project-chip · Jan 16, 2025 · 5055cbe · 5055cbe
1 parent 92c88e4
commit 5055cbe
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 20 deletions.
diff --git a/src/app/AttributeReportBuilder.h b/src/app/AttributeReportBuilder.h
@@ -58,19 +58,18 @@ class AttributeReportBuilder
     /**
      * EncodeValue encodes the value field of the report, it should be called exactly once.
      */
-    template <typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true, typename... Ts>
-    CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, TLV::Tag tag, T && item, Ts &&... aArgs)
+    template <typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
+    CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, TLV::Tag tag, const T & item)
     {
-        return DataModel::Encode(*(aAttributeReportIBs.GetAttributeReport().GetAttributeData().GetWriter()), tag, item,
-                                 std::forward<Ts>(aArgs)...);
+        return DataModel::Encode(*(aAttributeReportIBs.GetAttributeReport().GetAttributeData().GetWriter()), tag, item);
     }
 
-    template <typename T, std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true, typename... Ts>
-    CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, TLV::Tag tag, FabricIndex accessingFabricIndex,
-                           T && item, Ts &&... aArgs)
+    template <typename T, std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true>
+    CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, TLV::Tag tag, const T & item,
+                           FabricIndex accessingFabricIndex)
     {
         return DataModel::EncodeForRead(*(aAttributeReportIBs.GetAttributeReport().GetAttributeData().GetWriter()), tag,
-                                        accessingFabricIndex, item, std::forward<Ts>(aArgs)...);
+                                        accessingFabricIndex, item);
     }
 };
 

diff --git a/src/app/AttributeValueEncoder.h b/src/app/AttributeValueEncoder.h
@@ -46,7 +46,7 @@ class AttributeValueEncoder
         ListEncodeHelper(AttributeValueEncoder & encoder) : mAttributeValueEncoder(encoder) {}
 
         template <typename T, std::enable_if_t<DataModel::IsFabricScoped<T>::value, bool> = true>
-        CHIP_ERROR Encode(T && aArg) const
+        CHIP_ERROR Encode(const T & aArg) const
         {
             VerifyOrReturnError(aArg.GetFabricIndex() != kUndefinedFabricIndex, CHIP_ERROR_INVALID_FABRIC_INDEX);
 
@@ -55,13 +55,13 @@ class AttributeValueEncoder
             VerifyOrReturnError(!mAttributeValueEncoder.mIsFabricFiltered ||
                                     aArg.GetFabricIndex() == mAttributeValueEncoder.AccessingFabricIndex(),
                                 CHIP_NO_ERROR);
-            return mAttributeValueEncoder.EncodeListItem(mAttributeValueEncoder.AccessingFabricIndex(), std::forward<T>(aArg));
+            return mAttributeValueEncoder.EncodeListItem(aArg, mAttributeValueEncoder.AccessingFabricIndex());
         }
 
         template <typename T, std::enable_if_t<!DataModel::IsFabricScoped<T>::value, bool> = true>
-        CHIP_ERROR Encode(T && aArg) const
+        CHIP_ERROR Encode(const T & aArg) const
         {
-            return mAttributeValueEncoder.EncodeListItem(std::forward<T>(aArg));
+            return mAttributeValueEncoder.EncodeListItem(aArg);
         }
 
     private:
@@ -163,8 +163,12 @@ class AttributeValueEncoder
     friend class ListEncodeHelper;
     friend class TestOnlyAttributeValueEncoderAccessor;
 
-    template <typename... Ts>
-    CHIP_ERROR EncodeListItem(Ts &&... aArgs)
+    // EncodeListItem may be given an extra FabricIndex argument as a second
+    // arg, or not.  Represent that via a parameter pack (which might be
+    // empty). In practice, for any given ItemType the extra arg is either there
+    // or not, so we don't get more template explosion due to aExtraArgs.
+    template <typename ItemType, typename... ExtraArgTypes>
+    CHIP_ERROR EncodeListItem(const ItemType & aItem, ExtraArgTypes &&... aExtraArgs)
     {
         // EncodeListItem must be called after EnsureListStarted(), thus mCurrentEncodingListIndex and
         // mEncodeState.mCurrentEncodingListIndex are not invalid values.
@@ -183,11 +187,12 @@ class AttributeValueEncoder
         {
             // Just encode a single item, with an anonymous tag.
             AttributeReportBuilder builder;
-            err = builder.EncodeValue(mAttributeReportIBsBuilder, TLV::AnonymousTag(), std::forward<Ts>(aArgs)...);
+            err = builder.EncodeValue(mAttributeReportIBsBuilder, TLV::AnonymousTag(), aItem,
+                                      std::forward<ExtraArgTypes>(aExtraArgs)...);
         }
         else
         {
-            err = EncodeAttributeReportIB(std::forward<Ts>(aArgs)...);
+            err = EncodeAttributeReportIB(aItem, std::forward<ExtraArgTypes>(aExtraArgs)...);
         }
         if (err != CHIP_NO_ERROR)
         {
@@ -211,14 +216,19 @@ class AttributeValueEncoder
      * In particular, when we are encoding a single element in the list, mPath
      * must indicate a null list index to represent an "append" operation.
      * operation.
+     *
+     * EncodeAttributeReportIB may be given an extra FabricIndex argument as a second
+     * arg, or not.  Represent that via a parameter pack (which might be
+     * empty). In practice, for any given ItemType the extra arg is either
+     * there or not, so we don't get more template explosion due to aExtraArgs.
      */
-    template <typename... Ts>
-    CHIP_ERROR EncodeAttributeReportIB(Ts &&... aArgs)
+    template <typename ItemType, typename... ExtraArgTypes>
+    CHIP_ERROR EncodeAttributeReportIB(const ItemType & aItem, ExtraArgTypes &&... aExtraArgs)
     {
         AttributeReportBuilder builder;
         ReturnErrorOnFailure(builder.PrepareAttribute(mAttributeReportIBsBuilder, mPath, mDataVersion));
-        ReturnErrorOnFailure(builder.EncodeValue(mAttributeReportIBsBuilder, TLV::ContextTag(AttributeDataIB::Tag::kData),
-                                                 std::forward<Ts>(aArgs)...));
+        ReturnErrorOnFailure(builder.EncodeValue(mAttributeReportIBsBuilder, TLV::ContextTag(AttributeDataIB::Tag::kData), aItem,
+                                                 std::forward<ExtraArgTypes>(aExtraArgs)...));
 
         return builder.FinishAttribute(mAttributeReportIBsBuilder);
     }