From a087ade6d1b4ec4763215bcc95345a6898cb3191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 2 Dec 2024 16:55:03 -0800 Subject: [PATCH 1/2] improve value iterators, delete unnecessary code --- interpreter/for_test.go | 42 +++++++++++++----------- interpreter/value.go | 1 - interpreter/value_array.go | 31 ----------------- interpreter/value_ephemeral_reference.go | 6 ---- interpreter/value_storage_reference.go | 5 --- interpreter/value_string.go | 6 ++-- 6 files changed, 26 insertions(+), 65 deletions(-) diff --git a/interpreter/for_test.go b/interpreter/for_test.go index 48246d29b..4afc0baeb 100644 --- a/interpreter/for_test.go +++ b/interpreter/for_test.go @@ -862,25 +862,29 @@ func TestInclusiveRangeForInLoop(t *testing.T) { ) count := 0 - iterator := (loopElements).(*interpreter.ArrayValue).Iterator(inter, interpreter.EmptyLocationRange) - for { - elem := iterator.Next(inter, interpreter.EmptyLocationRange) - if elem == nil { - break - } - - AssertValuesEqual( - t, - inter, - interpreter.GetSmallIntegerValue( - int8(testCase.loopElements[count]), - integerStaticType, - ), - elem, - ) - - count += 1 - } + loopElementsArray := loopElements.(*interpreter.ArrayValue) + + loopElementsArray.ForEach( + inter, + nil, + func(value interpreter.Value) (resume bool) { + AssertValuesEqual( + t, + inter, + interpreter.GetSmallIntegerValue( + int8(testCase.loopElements[count]), + integerStaticType, + ), + value, + ) + + count += 1 + + return true + }, + false, + interpreter.EmptyLocationRange, + ) assert.Equal(t, len(testCase.loopElements), count) }) diff --git a/interpreter/value.go b/interpreter/value.go index 54756dff7..cf47e9c26 100644 --- a/interpreter/value.go +++ b/interpreter/value.go @@ -231,7 +231,6 @@ type ContractValue interface { // IterableValue is a value which can be iterated over, e.g. with a for-loop type IterableValue interface { Value - Iterator(interpreter *Interpreter, locationRange LocationRange) ValueIterator ForEach( interpreter *Interpreter, elementType sema.Type, diff --git a/interpreter/value_array.go b/interpreter/value_array.go index 5e030f36d..4734b185d 100644 --- a/interpreter/value_array.go +++ b/interpreter/value_array.go @@ -41,37 +41,6 @@ type ArrayValue struct { isDestroyed bool } -type ArrayValueIterator struct { - atreeIterator atree.ArrayIterator -} - -func (v *ArrayValue) Iterator(_ *Interpreter, _ LocationRange) ValueIterator { - arrayIterator, err := v.array.Iterator() - if err != nil { - panic(errors.NewExternalError(err)) - } - return ArrayValueIterator{ - atreeIterator: arrayIterator, - } -} - -var _ ValueIterator = ArrayValueIterator{} - -func (i ArrayValueIterator) Next(context ValueIteratorContext, _ LocationRange) Value { - atreeValue, err := i.atreeIterator.Next() - if err != nil { - panic(errors.NewExternalError(err)) - } - - if atreeValue == nil { - return nil - } - - // atree.Array iterator returns low-level atree.Value, - // convert to high-level interpreter.Value - return MustConvertStoredValue(context, atreeValue) -} - func NewArrayValue( interpreter *Interpreter, locationRange LocationRange, diff --git a/interpreter/value_ephemeral_reference.go b/interpreter/value_ephemeral_reference.go index e4643273a..3e7bd4d35 100644 --- a/interpreter/value_ephemeral_reference.go +++ b/interpreter/value_ephemeral_reference.go @@ -22,7 +22,6 @@ import ( "github.com/onflow/atree" "github.com/onflow/cadence/common" - "github.com/onflow/cadence/errors" "github.com/onflow/cadence/sema" ) @@ -335,11 +334,6 @@ func (*EphemeralReferenceValue) DeepRemove(_ *Interpreter, _ bool) { func (*EphemeralReferenceValue) isReference() {} -func (v *EphemeralReferenceValue) Iterator(_ *Interpreter, _ LocationRange) ValueIterator { - // Not used for now - panic(errors.NewUnreachableError()) -} - func (v *EphemeralReferenceValue) ForEach( interpreter *Interpreter, elementType sema.Type, diff --git a/interpreter/value_storage_reference.go b/interpreter/value_storage_reference.go index 447268aa1..6a60f5c93 100644 --- a/interpreter/value_storage_reference.go +++ b/interpreter/value_storage_reference.go @@ -412,11 +412,6 @@ func (*StorageReferenceValue) DeepRemove(_ *Interpreter, _ bool) { func (*StorageReferenceValue) isReference() {} -func (v *StorageReferenceValue) Iterator(_ *Interpreter, _ LocationRange) ValueIterator { - // Not used for now - panic(errors.NewUnreachableError()) -} - func (v *StorageReferenceValue) ForEach( interpreter *Interpreter, elementType sema.Type, diff --git a/interpreter/value_string.go b/interpreter/value_string.go index c709714fe..235a69603 100644 --- a/interpreter/value_string.go +++ b/interpreter/value_string.go @@ -635,7 +635,7 @@ func (v *StringValue) Split(inter *Interpreter, locationRange LocationRange, sep // Explode returns a Cadence array of type [String], where each element is a single character of the string func (v *StringValue) Explode(inter *Interpreter, locationRange LocationRange) *ArrayValue { - iterator := v.Iterator(inter, locationRange) + iterator := v.Iterator() return NewArrayValueWithIterator( inter, @@ -830,7 +830,7 @@ func (v *StringValue) ConformsToStaticType( return true } -func (v *StringValue) Iterator(_ *Interpreter, _ LocationRange) ValueIterator { +func (v *StringValue) Iterator() StringValueIterator { return StringValueIterator{ graphemes: uniseg.NewGraphemes(v.Str), } @@ -843,7 +843,7 @@ func (v *StringValue) ForEach( transferElements bool, locationRange LocationRange, ) { - iterator := v.Iterator(interpreter, locationRange) + iterator := v.Iterator() for { value := iterator.Next(interpreter, locationRange) if value == nil { From 653bc7ac73a7b15fd9b1b9627c633873ba5c0ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 2 Dec 2024 17:37:31 -0800 Subject: [PATCH 2/2] simplify ephemeral reference tracking --- interpreter/interpreter.go | 8 +++----- interpreter/value_composite.go | 1 - 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/interpreter/interpreter.go b/interpreter/interpreter.go index 0417cf2ab..d1896f5ec 100644 --- a/interpreter/interpreter.go +++ b/interpreter/interpreter.go @@ -5340,11 +5340,9 @@ func (interpreter *Interpreter) ValidateAtreeValue(value atree.Value) { } } -func (interpreter *Interpreter) maybeTrackReferencedResourceKindedValue(value Value) { - if referenceValue, ok := value.(*EphemeralReferenceValue); ok { - if value, ok := referenceValue.Value.(ReferenceTrackedResourceKindedValue); ok { - interpreter.trackReferencedResourceKindedValue(value.ValueID(), referenceValue) - } +func (interpreter *Interpreter) maybeTrackReferencedResourceKindedValue(referenceValue *EphemeralReferenceValue) { + if value, ok := referenceValue.Value.(ReferenceTrackedResourceKindedValue); ok { + interpreter.trackReferencedResourceKindedValue(value.ValueID(), referenceValue) } } diff --git a/interpreter/value_composite.go b/interpreter/value_composite.go index d3e649661..2af17d0f6 100644 --- a/interpreter/value_composite.go +++ b/interpreter/value_composite.go @@ -1775,7 +1775,6 @@ func (v *CompositeValue) forEachAttachment( // We create a reference here for the purposes of tracking it during iteration. vType := interpreter.MustSemaTypeOfValue(v) compositeReference := NewEphemeralReferenceValue(interpreter, UnauthorizedAccess, v, vType, locationRange) - interpreter.maybeTrackReferencedResourceKindedValue(compositeReference) forEachAttachment(interpreter, compositeReference, locationRange, f) }