VariantImpl: move addValue() to JsonArray

This way VariantImpl doesn't need to support generic types, which removes the dependency from `VariantImpl` to `JsonVariant`.

Author: bblanchon

src/ArduinoJson/Array/ArrayImpl.hpp

@@ -21,16 +21,16 @@ inline VariantImpl::iterator VariantImpl::at(size_t index) const {
   return it;
 }
 
-inline VariantData* VariantImpl::addElement(VariantData* data,
-                                            ResourceManager* resources) {
+inline VariantData* VariantImpl::addNewElement(VariantData* data,
+                                               ResourceManager* resources) {
   ARDUINOJSON_ASSERT(data != nullptr);
   ARDUINOJSON_ASSERT(data->isArray());
   ARDUINOJSON_ASSERT(resources != nullptr);
 
   auto slot = resources->allocVariant();
   if (!slot)
     return nullptr;
-  appendOne(slot, data, resources);
+  addElement(slot, data, resources);
   return slot.ptr();
 }
 
@@ -44,7 +44,7 @@ inline VariantData* VariantImpl::getOrAddElement(size_t index) {
     index++;
   VariantData* element = it.data();
   while (index > 0) {
-    element = addElement();
+    element = addNewElement();
     if (!element)
       return nullptr;
     index--;
@@ -60,23 +60,6 @@ inline void VariantImpl::removeElement(size_t index) {
   removeElement(at(index));
 }
 
-template <typename T>
-inline bool VariantImpl::addValue(const T& value, VariantData* data,
-                                  ResourceManager* resources) {
-  ARDUINOJSON_ASSERT(data != nullptr);
-  ARDUINOJSON_ASSERT(resources != nullptr);
-  auto slot = resources->allocVariant();
-  if (!slot)
-    return false;
-  JsonVariant variant(slot.ptr(), resources);
-  if (!variant.set(value)) {
-    freeVariant(slot, resources);
-    return false;
-  }
-  appendOne(slot, data, resources);
-  return true;
-}
-
 // Returns the size (in bytes) of an array with n elements.
 constexpr size_t sizeofArray(size_t n) {
   return n * sizeof(VariantData);

src/ArduinoJson/Array/JsonArray.hpp

@@ -56,22 +56,26 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
   template <typename T, detail::enable_if_t<
                             detail::is_same<T, JsonVariant>::value, int> = 0>
   JsonVariant add() const {
-    return JsonVariant(impl_.addElement(), impl_.getResourceManager());
+    return JsonVariant(impl_.addNewElement(), impl_.getResourceManager());
   }
 
   // Appends a value to the array.
   // https://arduinojson.org/v7/api/jsonarray/add/
   template <typename T>
   bool add(const T& value) const {
-    return impl_.addValue(value);
+    if (!impl_.isArray())
+      return false;
+    return addValue(value, impl_.getData(), impl_.getResourceManager());
   }
 
   // Appends a value to the array.
   // https://arduinojson.org/v7/api/jsonarray/add/
   template <typename T,
             detail::enable_if_t<!detail::is_const<T>::value, int> = 0>
   bool add(T* value) const {
-    return impl_.addValue(value);
+    if (!impl_.isArray())
+      return false;
+    return addValue(value, impl_.getData(), impl_.getResourceManager());
   }
 
   // Returns an iterator to the first element of the array.
@@ -207,6 +211,28 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
     return impl_.getData();
   }
 
+  // HACK: this function has been pulled out of VariantImpl to avoid the
+  // circular dependency between VariantImpl and JsonVariant
+  template <typename T>
+  static bool addValue(const T& value, detail::VariantData* data,
+                       detail::ResourceManager* resources) {
+    ARDUINOJSON_ASSERT(data != nullptr);
+    ARDUINOJSON_ASSERT(data->isArray());
+    ARDUINOJSON_ASSERT(resources != nullptr);
+
+    auto slot = resources->allocVariant();
+    if (!slot)
+      return false;
+
+    if (!JsonVariant(slot.ptr(), resources).set(value)) {
+      detail::VariantImpl::freeVariant(slot, resources);
+      return false;
+    }
+
+    detail::VariantImpl::addElement(slot, data, resources);
+    return true;
+  }
+
   mutable detail::VariantImpl impl_;
 };
 

src/ArduinoJson/Collection/CollectionImpl.hpp

@@ -24,8 +24,8 @@ inline VariantImpl::iterator VariantImpl::createIterator(
   return iterator(resources->getVariant(head), head);
 }
 
-inline void VariantImpl::appendOne(Slot<VariantData> slot, VariantData* data,
-                                   ResourceManager* resources) {
+inline void VariantImpl::addElement(Slot<VariantData> slot, VariantData* data,
+                                    ResourceManager* resources) {
   ARDUINOJSON_ASSERT(data != nullptr);
   ARDUINOJSON_ASSERT(data->isCollection());
   ARDUINOJSON_ASSERT(resources != nullptr);

src/ArduinoJson/Document/JsonDocument.hpp

@@ -287,22 +287,22 @@ class JsonDocument : public detail::VariantOperators<const JsonDocument&> {
   template <typename T, detail::enable_if_t<
                             detail::is_same<T, JsonVariant>::value, int> = 0>
   JsonVariant add() {
-    return JsonVariant(getOrCreateArray().addElement(), &resources_);
+    return getOrCreateArray().add<T>();
   }
 
   // Appends a value to the root array.
   // https://arduinojson.org/v7/api/jsondocument/add/
   template <typename TValue>
   bool add(const TValue& value) {
-    return getOrCreateArray().addValue(value);
+    return getOrCreateArray().add(value);
   }
 
   // Appends a value to the root array.
   // https://arduinojson.org/v7/api/jsondocument/add/
   template <typename TChar,
             detail::enable_if_t<!detail::is_const<TChar>::value, int> = 0>
   bool add(TChar* value) {
-    return getOrCreateArray().addValue(value);
+    return getOrCreateArray().add(value);
   }
 
   // Removes an element of the root array.
@@ -412,10 +412,10 @@ class JsonDocument : public detail::VariantOperators<const JsonDocument&> {
     return detail::VariantImpl(&data_, &resources_);
   }
 
-  detail::VariantImpl getOrCreateArray() const {
+  JsonArray getOrCreateArray() const {
     if (data_.type == detail::VariantType::Null)
       data_.toArray();
-    return detail::VariantImpl(&data_, &resources_);
+    return JsonArray(&data_, &resources_);
   }
 
   JsonVariant getVariant() {

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -174,7 +174,7 @@ class JsonDeserializer {
     for (;;) {
       if (elementFilter.allow()) {
         // Allocate slot in array
-        VariantData* value = VariantImpl::addElement(array, resources_);
+        VariantData* value = VariantImpl::addNewElement(array, resources_);
         if (!value)
           return DeserializationError::NoMemory;
 

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -360,7 +360,7 @@ class MsgPackDeserializer {
       VariantData* value;
 
       if (elementFilter.allow()) {
-        value = VariantImpl::addElement(variant, resources_);
+        value = VariantImpl::addNewElement(variant, resources_);
         if (!value)
           return DeserializationError::NoMemory;
       } else {

src/ArduinoJson/Variant/VariantImpl.hpp

@@ -96,13 +96,16 @@ class VariantImpl {
     }
   }
 
-  VariantData* addElement() {
+  VariantData* addNewElement() {
     if (!isArray())
       return nullptr;
-    return addElement(data_, resources_);
+    return addNewElement(data_, resources_);
   }
 
-  static VariantData* addElement(VariantData*, ResourceManager*);
+  static VariantData* addNewElement(VariantData*, ResourceManager*);
+
+  static void addElement(Slot<VariantData> slot, VariantData*,
+                         ResourceManager*);
 
   template <typename TAdaptedString>
   VariantData* addMember(TAdaptedString key) {
@@ -124,16 +127,6 @@ class VariantImpl {
   static VariantData* addPair(VariantData** value, VariantData*,
                               ResourceManager*);
 
-  template <typename T>
-  bool addValue(const T& value) {
-    if (!isArray())
-      return false;
-    return addValue(value, data_, resources_);
-  }
-
-  template <typename T>
-  static bool addValue(const T& value, VariantData*, ResourceManager*);
-
   bool asBoolean() const {
     return asBoolean(data_, resources_);
   }
@@ -613,6 +606,11 @@ class VariantImpl {
 
   static void empty(VariantData*, ResourceManager*);
 
+  static void freeVariant(Slot<VariantData> slot, ResourceManager* resources) {
+    clear(slot.ptr(), resources);
+    resources->freeVariant(slot);
+  }
+
  private:
   VariantData* data_;
   ResourceManager* resources_;
@@ -627,19 +625,12 @@ class VariantImpl {
   template <typename TAdaptedString>
   static iterator findKey(TAdaptedString key, VariantData*, ResourceManager*);
 
-  static void appendOne(Slot<VariantData> slot, VariantData*, ResourceManager*);
-
   static void appendPair(Slot<VariantData> key, Slot<VariantData> value,
                          VariantData*, ResourceManager*);
 
   void removeOne(iterator it);
   void removePair(iterator it);
 
-  static void freeVariant(Slot<VariantData> slot, ResourceManager* resources) {
-    clear(slot.ptr(), resources);
-    resources->freeVariant(slot);
-  }
-
   Slot<VariantData> getPreviousSlot(VariantData*) const;
 };
 

src/ArduinoJson/Variant/VariantRefBase.hpp

@@ -119,14 +119,14 @@ class VariantRefBase : public VariantTag {
   // https://arduinojson.org/v7/api/jsonvariant/add/
   template <typename T>
   bool add(const T& value) const {
-    return getOrCreateArray().addValue(value);
+    return getOrCreateArray().add(value);
   }
 
   // Appends a value to the array.
   // https://arduinojson.org/v7/api/jsonvariant/add/
   template <typename T, enable_if_t<!is_const<T>::value, int> = 0>
   bool add(T* value) const {
-    return getOrCreateArray().addValue(value);
+    return getOrCreateArray().add(value);
   }
 
   // Removes an element of the array.
@@ -278,13 +278,7 @@ class VariantRefBase : public VariantTag {
     return VariantImpl(getOrCreateData(), getResourceManager());
   }
 
-  VariantImpl getOrCreateArray() const {
-    auto data = getOrCreateData();
-    auto resources = getResourceManager();
-    if (data && data->type == VariantType::Null)
-      data->toArray();
-    return VariantImpl(data, resources);
-  }
+  JsonArray getOrCreateArray() const;
 
   FORCE_INLINE ArduinoJson::JsonVariant getVariant() const;
 

src/ArduinoJson/Variant/VariantRefBaseImpl.hpp

@@ -69,7 +69,7 @@ inline void convertToJson(const VariantRefBase<TDerived>& src,
 template <typename TDerived>
 template <typename T, enable_if_t<is_same<T, JsonVariant>::value, int>>
 inline T VariantRefBase<TDerived>::add() const {
-  return JsonVariant(getOrCreateArray().addElement(), getResourceManager());
+  return getOrCreateArray().template add<T>();
 }
 
 template <typename TDerived>
@@ -95,6 +95,15 @@ inline JsonVariant VariantRefBase<TDerived>::getVariant() const {
   return JsonVariant(getData(), getResourceManager());
 }
 
+template <typename TDerived>
+inline JsonArray VariantRefBase<TDerived>::getOrCreateArray() const {
+  auto data = getOrCreateData();
+  auto resources = getResourceManager();
+  if (data && data->type == VariantType::Null)
+    data->toArray();
+  return JsonArray(data, resources);
+}
+
 template <typename TDerived>
 inline JsonVariant VariantRefBase<TDerived>::getOrCreateVariant() const {
   return JsonVariant(getOrCreateData(), getResourceManager());