From ff758a53b41bce64dd02c043cf85d7f76e5540e0 Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Thu, 21 Jan 2021 11:02:46 +0000
Subject: [PATCH] Array:  Fix perfect forwarding when adding elements

---
 modules/juce_core/containers/juce_Array.h     | 16 +++----
 modules/juce_core/containers/juce_ArrayBase.h | 45 +++++++------------
 modules/juce_core/text/juce_StringArray.h     |  3 +-
 3 files changed, 27 insertions(+), 37 deletions(-)

diff --git a/modules/juce_core/containers/juce_Array.h b/modules/juce_core/containers/juce_Array.h
index 85f833b123..64013f11db 100644
--- a/modules/juce_core/containers/juce_Array.h
+++ b/modules/juce_core/containers/juce_Array.h
@@ -110,16 +110,16 @@ public:
 
     /** Initalises an Array from a list of items. */
     template <typename... OtherElements>
-    Array (const ElementType& firstNewElement, OtherElements... otherElements)
+    Array (const ElementType& firstNewElement, OtherElements&&... otherElements)
     {
-        values.add (firstNewElement, otherElements...);
+        values.add (firstNewElement, std::forward<OtherElements> (otherElements)...);
     }
 
     /** Initalises an Array from a list of items. */
     template <typename... OtherElements>
-    Array (ElementType&& firstNewElement, OtherElements... otherElements)
+    Array (ElementType&& firstNewElement, OtherElements&&... otherElements)
     {
-        values.add (std::move (firstNewElement), otherElements...);
+        values.add (std::move (firstNewElement), std::forward<OtherElements> (otherElements)...);
     }
 
     template <typename TypeToCreateFrom>
@@ -433,18 +433,18 @@ public:
 
     /** Appends multiple new elements at the end of the array. */
     template <typename... OtherElements>
-    void add (const ElementType& firstNewElement, OtherElements... otherElements)
+    void add (const ElementType& firstNewElement, OtherElements&&... otherElements)
     {
         const ScopedLockType lock (getLock());
-        values.add (firstNewElement, otherElements...);
+        values.add (firstNewElement, std::forward<OtherElements> (otherElements)...);
     }
 
     /** Appends multiple new elements at the end of the array. */
     template <typename... OtherElements>
-    void add (ElementType&& firstNewElement, OtherElements... otherElements)
+    void add (ElementType&& firstNewElement, OtherElements&&... otherElements)
     {
         const ScopedLockType lock (getLock());
-        values.add (std::move (firstNewElement), otherElements...);
+        values.add (std::move (firstNewElement), std::forward<OtherElements> (otherElements)...);
     }
 
     /** Inserts a new element into the array at a given position.
diff --git a/modules/juce_core/containers/juce_ArrayBase.h b/modules/juce_core/containers/juce_ArrayBase.h
index 7d5c4d0b5b..af39b2ed7d 100644
--- a/modules/juce_core/containers/juce_ArrayBase.h
+++ b/modules/juce_core/containers/juce_ArrayBase.h
@@ -255,32 +255,24 @@ public:
     //==============================================================================
     void add (const ElementType& newElement)
     {
-        checkSourceIsNotAMember (&newElement);
-        ensureAllocatedSize (numUsed + 1);
-        addAssumingCapacityIsReady (newElement);
+        addImpl (newElement);
     }
 
     void add (ElementType&& newElement)
     {
-        checkSourceIsNotAMember (&newElement);
-        ensureAllocatedSize (numUsed + 1);
-        addAssumingCapacityIsReady (std::move (newElement));
+        addImpl (std::move (newElement));
     }
 
     template <typename... OtherElements>
-    void add (const ElementType& firstNewElement, OtherElements... otherElements)
+    void add (const ElementType& firstNewElement, OtherElements&&... otherElements)
     {
-        checkSourceIsNotAMember (&firstNewElement);
-        ensureAllocatedSize (numUsed + 1 + (int) sizeof... (otherElements));
-        addAssumingCapacityIsReady (firstNewElement, otherElements...);
+        addImpl (firstNewElement, std::forward<OtherElements> (otherElements)...);
     }
 
     template <typename... OtherElements>
-    void add (ElementType&& firstNewElement, OtherElements... otherElements)
+    void add (ElementType&& firstNewElement, OtherElements&&... otherElements)
     {
-        checkSourceIsNotAMember (&firstNewElement);
-        ensureAllocatedSize (numUsed + 1 + (int) sizeof... (otherElements));
-        addAssumingCapacityIsReady (std::move (firstNewElement), otherElements...);
+        addImpl (std::move (firstNewElement), std::forward<OtherElements> (otherElements)...);
     }
 
     //==============================================================================
@@ -335,7 +327,7 @@ public:
     //==============================================================================
     void insert (int indexToInsertAt, ParameterType newElement, int numberOfTimesToInsertIt)
     {
-        checkSourceIsNotAMember (&newElement);
+        checkSourceIsNotAMember (newElement);
         auto* space = createInsertSpace (indexToInsertAt, numberOfTimesToInsertIt);
 
         for (int i = 0; i < numberOfTimesToInsertIt; ++i)
@@ -561,21 +553,18 @@ private:
     }
 
     //==============================================================================
-    void addAssumingCapacityIsReady (const ElementType& element)   { new (elements + numUsed++) ElementType (element); }
-    void addAssumingCapacityIsReady (ElementType&& element)        { new (elements + numUsed++) ElementType (std::move (element)); }
-
-    template <typename... OtherElements>
-    void addAssumingCapacityIsReady (const ElementType& firstNewElement, OtherElements... otherElements)
+    template <typename... Elements>
+    void addImpl (Elements&&... toAdd)
     {
-        addAssumingCapacityIsReady (firstNewElement);
-        addAssumingCapacityIsReady (otherElements...);
+        ignoreUnused (std::initializer_list<int> { (((void) checkSourceIsNotAMember (toAdd)), 0)... });
+        ensureAllocatedSize (numUsed + (int) sizeof... (toAdd));
+        addAssumingCapacityIsReady (std::forward<Elements> (toAdd)...);
     }
 
-    template <typename... OtherElements>
-    void addAssumingCapacityIsReady (ElementType&& firstNewElement, OtherElements... otherElements)
+    template <typename... Elements>
+    void addAssumingCapacityIsReady (Elements&&... toAdd)
     {
-        addAssumingCapacityIsReady (std::move (firstNewElement));
-        addAssumingCapacityIsReady (otherElements...);
+        ignoreUnused (std::initializer_list<int> { ((void) (new (elements + numUsed++) ElementType (std::forward<Elements> (toAdd))), 0)... });
     }
 
     //==============================================================================
@@ -594,14 +583,14 @@ private:
         new (destination) ElementType (std::move (source));
     }
 
-    void checkSourceIsNotAMember (const ElementType* element)
+    void checkSourceIsNotAMember (const ElementType& element)
     {
         // when you pass a reference to an existing element into a method like add() which
         // may need to reallocate the array to make more space, the incoming reference may
         // be deleted indirectly during the reallocation operation! To work around this,
         // make a local copy of the item you're trying to add (and maybe use std::move to
         // move it into the add() method to avoid any extra overhead)
-        jassert (element < begin() || element >= end());
+        jassert (std::addressof (element) < begin() || end() <= std::addressof (element));
         ignoreUnused (element);
     }
 
diff --git a/modules/juce_core/text/juce_StringArray.h b/modules/juce_core/text/juce_StringArray.h
index ac597b748e..4421d5cc0e 100644
--- a/modules/juce_core/text/juce_StringArray.h
+++ b/modules/juce_core/text/juce_StringArray.h
@@ -49,7 +49,8 @@ public:
 
     /** Creates an array containing a list of strings. */
     template <typename... OtherElements>
-    StringArray (StringRef firstValue, OtherElements... otherValues) : strings (firstValue, otherValues...) {}
+    StringArray (StringRef firstValue, OtherElements&&... otherValues)
+        : strings (firstValue, std::forward<OtherElements> (otherValues)...) {}
 
     /** Creates an array containing a list of strings. */
     StringArray (const std::initializer_list<const char*>& strings);