Fix for longstanding bug where set -e would fail to erase elements from an array.

This was introduced in 7b3377e78c

builtin_set.cpp

@@ -275,23 +275,21 @@ static int update_values( wcstring_list_t &list,
 /**
    Erase from a list of wcstring values at specified indexes 
 */
-static void erase_values(wcstring_list_t &list, std::vector<long> &indexes) 
+static void erase_values(wcstring_list_t &list, const std::vector<long> &indexes) 
 {
-	size_t i;
-	wcstring_list_t result;
-	for (i = 0; i < list.size(); i++) 
-	{
-		if (std::find(indexes.begin(), indexes.end(),  i + 1) != indexes.end()) 
-		{
-			result.push_back( list[ i ] );
-		}
-	}
-	
-//	al_truncate(list,0);	
-	list.clear();
-	copy(result.begin(),result.end(),back_inserter( list ) );
-
-//	al_destroy(&result);
+    // Make a set of indexes.
+    // This both sorts them into ascending order and removes duplicates.
+    const std::set<long> indexes_set(indexes.begin(), indexes.end());
+    
+    // Now walk the set backwards, so we encounter larger indexes first, and remove elements at the given (1-based) indexes.
+    std::set<long>::const_reverse_iterator iter;
+    for (iter = indexes_set.rbegin(); iter != indexes_set.rend(); iter++) {
+        long val = *iter;
+        if (val > 0 && val <= list.size()) {
+            // One-based indexing!
+            list.erase(list.begin() + val - 1);
+        }
+    }
 }
 
 

tests/test3.in

@@ -127,3 +127,10 @@ else
 end
 
 
+set foo abc def
+set -e foo[1]
+if test $foo '=' def
+	echo Test 11 pass
+else
+	echo Test 11 fail
+end

tests/test3.out

@@ -8,3 +8,4 @@ Test 7 pass
 Test 8 pass
 Test 9 pass
 Test 10 pass
+Test 11 pass