Use atree readonly iterators when feasible

onflow · Mar 12, 2024 · 3f70345 · 3f70345
1 parent 6713e43
commit 3f70345
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 22 deletions.
diff --git a/runtime/interpreter/interpreter.go b/runtime/interpreter/interpreter.go
@@ -2061,7 +2061,7 @@ func (interpreter *Interpreter) convert(value Value, valueType, targetType sema.
 
 			array := arrayValue.array
 
-			iterator, err := array.Iterator()
+			iterator, err := array.ReadOnlyIterator()
 			if err != nil {
 				panic(errors.NewExternalError(err))
 			}
@@ -2102,10 +2102,7 @@ func (interpreter *Interpreter) convert(value Value, valueType, targetType sema.
 
 			dictionary := dictValue.dictionary
 
-			valueComparator := newValueComparator(interpreter, locationRange)
-			hashInputProvider := newHashInputProvider(interpreter, locationRange)
-
-			iterator, err := dictionary.Iterator(valueComparator, hashInputProvider)
+			iterator, err := dictionary.ReadOnlyIterator()
 			if err != nil {
 				panic(errors.NewExternalError(err))
 			}

diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go
@@ -1973,12 +1973,14 @@ func (v *ArrayValue) Concat(interpreter *Interpreter, locationRange LocationRang
 
 	first := true
 
-	firstIterator, err := v.array.Iterator()
+	// Use ReadOnlyIterator here because new ArrayValue is created with elements copied (not removed) from original value.
+	firstIterator, err := v.array.ReadOnlyIterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
 	}
 
-	secondIterator, err := other.array.Iterator()
+	// Use ReadOnlyIterator here because new ArrayValue is created with elements copied (not removed) from original value.
+	secondIterator, err := other.array.ReadOnlyIterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
 	}
@@ -3075,7 +3077,8 @@ func (v *ArrayValue) Slice(
 		})
 	}
 
-	iterator, err := v.array.RangeIterator(uint64(fromIndex), uint64(toIndex))
+	// Use ReadOnlyIterator here because new ArrayValue is created from elements copied (not removed) from original ArrayValue.
+	iterator, err := v.array.ReadOnlyRangeIterator(uint64(fromIndex), uint64(toIndex))
 	if err != nil {
 
 		var sliceOutOfBoundsError *atree.SliceOutOfBoundsError
@@ -3192,6 +3195,7 @@ func (v *ArrayValue) Filter(
 		return invocation
 	}
 
+	// TODO: Use ReadOnlyIterator here if procedure doesn't change array elements.
 	iterator, err := v.array.Iterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
@@ -3293,6 +3297,7 @@ func (v *ArrayValue) Map(
 		panic(errors.NewUnreachableError())
 	}
 
+	// TODO: Use ReadOnlyIterator here if procedure doesn't change map values.
 	iterator, err := v.array.Iterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
@@ -3357,7 +3362,8 @@ func (v *ArrayValue) ToVariableSized(
 		panic(errors.NewUnreachableError())
 	}
 
-	iterator, err := v.array.Iterator()
+	// Use ReadOnlyIterator here because ArrayValue elements are copied (not removed) from original ArrayValue.
+	iterator, err := v.array.ReadOnlyIterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
 	}
@@ -3417,7 +3423,8 @@ func (v *ArrayValue) ToConstantSized(
 		panic(errors.NewUnreachableError())
 	}
 
-	iterator, err := v.array.Iterator()
+	// Use ReadOnlyIterator here because ArrayValue elements are copied (not removed) from original ArrayValue.
+	iterator, err := v.array.ReadOnlyIterator()
 	if err != nil {
 		panic(errors.NewExternalError(err))
 	}
@@ -17936,9 +17943,7 @@ func (v *CompositeValue) ForEachFieldName(
 	f func(fieldName string) (resume bool),
 ) {
 	iterate := func(fn atree.MapElementIterationFunc) error {
-		return v.dictionary.IterateKeys(
-			StringAtreeValueComparator,
-			StringAtreeValueHashInput,
+		return v.dictionary.IterateReadOnlyKeys(
 			fn,
 		)
 	}
@@ -18633,12 +18638,8 @@ func (v *DictionaryValue) IterateKeys(
 	locationRange LocationRange,
 	f func(key Value) (resume bool),
 ) {
-	valueComparator := newValueComparator(interpreter, locationRange)
-	hashInputProvider := newHashInputProvider(interpreter, locationRange)
 	iterate := func(fn atree.MapElementIterationFunc) error {
-		return v.dictionary.IterateKeys(
-			valueComparator,
-			hashInputProvider,
+		return v.dictionary.IterateReadOnlyKeys(
 			fn,
 		)
 	}
@@ -19110,10 +19111,8 @@ func (v *DictionaryValue) GetMember(
 
 	case "values":
 
-		valueComparator := newValueComparator(interpreter, locationRange)
-		hashInputProvider := newHashInputProvider(interpreter, locationRange)
-
-		iterator, err := v.dictionary.Iterator(valueComparator, hashInputProvider)
+		// Use ReadOnlyIterator here because new ArrayValue is created with copied elements (not removed) from original.
+		iterator, err := v.dictionary.ReadOnlyIterator()
 		if err != nil {
 			panic(errors.NewExternalError(err))
 		}