Added assertions to Array to catch situations where a reference to a member is passed into an add() method. Also changed the form of StringArray method params to avoid this problem.

modules/juce_core/containers/juce_Array.h

@@ -1083,7 +1083,7 @@ public:
 
 private:
     //==============================================================================
-    ArrayBase <ElementType, TypeOfCriticalSectionToUse> values;
+    ArrayBase<ElementType, TypeOfCriticalSectionToUse> values;
 
     void removeInternal (int indexToRemove)
     {

modules/juce_core/containers/juce_ArrayBase.h

@@ -178,12 +178,14 @@ public:
     //==============================================================================
     void add (const ElementType& newElement)
     {
+        checkSourceIsNotAMember (&newElement);
         ensureAllocatedSize (numUsed + 1);
         addAssumingCapacityIsReady (newElement);
     }
 
     void add (ElementType&& newElement)
     {
+        checkSourceIsNotAMember (&newElement);
         ensureAllocatedSize (numUsed + 1);
         addAssumingCapacityIsReady (std::move (newElement));
     }
@@ -191,6 +193,7 @@ public:
     template <typename... OtherElements>
     void add (const ElementType& firstNewElement, OtherElements... otherElements)
     {
+        checkSourceIsNotAMember (&firstNewElement);
         ensureAllocatedSize (numUsed + 1 + (int) sizeof... (otherElements));
         addAssumingCapacityIsReady (firstNewElement, otherElements...);
     }
@@ -198,6 +201,7 @@ public:
     template <typename... OtherElements>
     void add (ElementType&& firstNewElement, OtherElements... otherElements)
     {
+        checkSourceIsNotAMember (&firstNewElement);
         ensureAllocatedSize (numUsed + 1 + (int) sizeof... (otherElements));
         addAssumingCapacityIsReady (std::move (firstNewElement), otherElements...);
     }
@@ -223,6 +227,7 @@ public:
     template <class OtherArrayType>
     void addArray (const OtherArrayType& arrayToAddFrom)
     {
+        jassert ((const void*) this != (const void*) &arrayToAddFrom); // can't add from our own elements!
         ensureAllocatedSize (numUsed + (int) arrayToAddFrom.size());
 
         for (auto& e : arrayToAddFrom)
@@ -234,6 +239,8 @@ public:
     addArray (const OtherArrayType& arrayToAddFrom,
               int startIndex, int numElementsToAdd = -1)
     {
+        jassert ((const void*) this != (const void*) &arrayToAddFrom); // can't add from our own elements!
+
         if (startIndex < 0)
         {
             jassertfalse;
@@ -251,6 +258,7 @@ public:
     //==============================================================================
     void insert (int indexToInsertAt, ParameterType newElement, int numberOfTimesToInsertIt)
     {
+        checkSourceIsNotAMember (&newElement);
         auto* space = createInsertSpace (indexToInsertAt, numberOfTimesToInsertIt);
 
         for (int i = 0; i < numberOfTimesToInsertIt; ++i)
@@ -502,6 +510,17 @@ private:
         new (destination) ElementType (std::move (source));
     }
 
+    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());
+        ignoreUnused (element);
+    }
+
     //==============================================================================
     HeapBlock<ElementType> elements;
     int numAllocated = 0, numUsed = 0;

modules/juce_core/text/juce_StringArray.cpp

@@ -132,19 +132,18 @@ String& StringArray::getReference (int index) noexcept
     return strings.getReference (index);
 }
 
-void StringArray::add (const String& newString)
+void StringArray::add (String newString)
 {
-    strings.add (newString);
+    // NB: the local temp copy is to avoid a dangling pointer if the
+    // argument being passed-in is a reference into this array.
+    strings.add (std::move (newString));
 }
 
-void StringArray::add (String&& stringToAdd)
+void StringArray::insert (int index, String newString)
 {
-    strings.add (static_cast<String&&> (stringToAdd));
-}
-
-void StringArray::insert (int index, const String& newString)
-{
-    strings.insert (index, newString);
+    // NB: the local temp copy is to avoid a dangling pointer if the
+    // argument being passed-in is a reference into this array.
+    strings.insert (index, std::move (newString));
 }
 
 bool StringArray::addIfNotAlreadyThere (const String& newString, bool ignoreCase)
@@ -158,6 +157,8 @@ bool StringArray::addIfNotAlreadyThere (const String& newString, bool ignoreCase
 
 void StringArray::addArray (const StringArray& otherArray, int startIndex, int numElementsToAdd)
 {
+    jassert (this != &otherArray); // can't add from our own elements!
+
     if (startIndex < 0)
     {
         jassertfalse;
@@ -173,13 +174,15 @@ void StringArray::addArray (const StringArray& otherArray, int startIndex, int n
 
 void StringArray::mergeArray (const StringArray& otherArray, bool ignoreCase)
 {
+    jassert (this != &otherArray); // can't add from our own elements!
+
     for (auto& s : otherArray)
         addIfNotAlreadyThere (s, ignoreCase);
 }
 
-void StringArray::set (int index, const String& newString)
+void StringArray::set (int index, String newString)
 {
-    strings.set (index, newString);
+    strings.set (index, std::move (newString));
 }
 
 bool StringArray::contains (StringRef stringToLookFor, bool ignoreCase) const

modules/juce_core/text/juce_StringArray.h

@@ -187,10 +187,7 @@ public:
 
     //==============================================================================
     /** Appends a string at the end of the array. */
-    void add (const String& stringToAdd);
-
-    /** Appends a string at the end of the array. */
-    void add (String&& stringToAdd);
+    void add (String stringToAdd);
 
     /** Inserts a string into the array.
 
@@ -199,7 +196,7 @@ public:
         If the index is less than zero or greater than the size of the array,
         the new string will be added to the end of the array.
     */
-    void insert (int index, const String& stringToAdd);
+    void insert (int index, String stringToAdd);
 
     /** Adds a string to the array as long as it's not already in there.
         The search can optionally be case-insensitive.
@@ -213,7 +210,7 @@ public:
         If the index is higher than the array's size, the new string will be
         added to the end of the array; if it's less than zero nothing happens.
     */
-    void set (int index, const String& newString);
+    void set (int index, String newString);
 
     /** Appends some strings from another array to the end of this one.
 

modules/juce_data_structures/values/juce_ValueWithDefault.h

@@ -209,6 +209,7 @@ private:
         jassert (delimiter.isNotEmpty());
 
         StringArray elements;
+
         for (auto& v : input)
             elements.add (v.toString());