From 974329343368cab2b9e3d432de83f8536c6be5d3 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 9 Jun 2023 14:58:38 -0700 Subject: [PATCH 01/24] Change member access result type to a reference --- runtime/sema/check_expression.go | 19 +- runtime/sema/check_member_expression.go | 42 +++- runtime/tests/checker/member_test.go | 243 ++++++++++++++++++++++++ 3 files changed, 301 insertions(+), 3 deletions(-) diff --git a/runtime/sema/check_expression.go b/runtime/sema/check_expression.go index d06fad80ae..77996b290c 100644 --- a/runtime/sema/check_expression.go +++ b/runtime/sema/check_expression.go @@ -258,7 +258,24 @@ func (checker *Checker) VisitStringExpression(expression *ast.StringExpression) } func (checker *Checker) VisitIndexExpression(expression *ast.IndexExpression) Type { - return checker.visitIndexExpression(expression, false) + elementType := checker.visitIndexExpression(expression, false) + if elementType == InvalidType { + return elementType + } + + indexExprTypes := checker.Elaboration.IndexExpressionTypes(expression) + parentType := indexExprTypes.IndexedType + + // If the element, + // 1) is accessed via a reference, and + // 2) is container-typed, + // then the element type should also be a reference. + if _, accessedViaReference := parentType.(*ReferenceType); accessedViaReference && + checker.isContainerType(elementType) { + elementType = checker.getReferenceType(elementType) + } + + return elementType } // visitIndexExpression checks if the indexed expression is indexable, diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 61cbf5b52f..b9bedff77c 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -85,16 +85,54 @@ func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) // If the member access is optional chaining, only wrap the result value // in an optional, if it is not already an optional value - if isOptional { if _, ok := memberType.(*OptionalType); !ok { - return &OptionalType{Type: memberType} + memberType = &OptionalType{Type: memberType} } } + // If the member, + // 1) is accessed via a reference, and + // 2) is container-typed, + // then the member type should also be a reference. + if _, accessedViaReference := accessedType.(*ReferenceType); accessedViaReference && + member.DeclarationKind == common.DeclarationKindField && + checker.isContainerType(memberType) { + // Get a reference to the type + memberType = checker.getReferenceType(memberType) + } + return memberType } +// getReferenceType Returns a reference type to a given type. +// Reference to an optional should return an optional reference. +// This has to be done recursively for nested optionals. +// e.g.1: Given type T, this method returns &T. +// e.g.2: Given T?, this returns (&T)? +func (checker *Checker) getReferenceType(typ Type) Type { + if optionalType, ok := typ.(*OptionalType); ok { + return &OptionalType{ + Type: checker.getReferenceType(optionalType.Type), + } + } + + return NewReferenceType(checker.memoryGauge, typ, false) +} + +func (checker *Checker) isContainerType(typ Type) bool { + switch typ := typ.(type) { + case *CompositeType, + *DictionaryType, + ArrayType: + return true + case *OptionalType: + return checker.isContainerType(typ.Type) + default: + return false + } +} + func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedType Type, member *Member, isOptional bool) { memberInfo, ok := checker.Elaboration.MemberExpressionMemberInfo(expression) if ok { diff --git a/runtime/tests/checker/member_test.go b/runtime/tests/checker/member_test.go index 2221b31a63..bd5c866aa4 100644 --- a/runtime/tests/checker/member_test.go +++ b/runtime/tests/checker/member_test.go @@ -418,3 +418,246 @@ func TestCheckMemberNotDeclaredSecondaryError(t *testing.T) { assert.Equal(t, "unknown member", memberErr.SecondaryError()) }) } + +func TestCheckMemberAccess(t *testing.T) { + + t.Parallel() + + t.Run("composite, field", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + var x: [Int] + init() { + self.x = [] + } + } + + fun test() { + let test = Test() + var x: [Int] = test.x + } + `) + + require.NoError(t, err) + }) + + t.Run("composite, function", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + pub fun foo(): Int { + return 1 + } + } + + fun test() { + let test = Test() + var foo: (fun(): Int) = test.foo + } + `) + + require.NoError(t, err) + }) + + t.Run("composite reference, field", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + var x: [Int] + init() { + self.x = [] + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: &[Int] = testRef.x + } + `) + + require.NoError(t, err) + }) + + t.Run("composite reference, optional field", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + var x: [Int]? + init() { + self.x = [] + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: &[Int]? = testRef.x + } + `) + + require.NoError(t, err) + }) + + t.Run("composite reference, primitive field", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + var x: Int + init() { + self.x = 1 + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: Int = testRef.x + } + `) + + require.NoError(t, err) + }) + + t.Run("composite reference, function", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct Test { + pub fun foo(): Int { + return 1 + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var foo: (fun(): Int) = testRef.foo + } + `) + + require.NoError(t, err) + }) + + t.Run("array, element", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let array: [[Int]] = [[1, 2]] + var x: [Int] = array[0] + } + `) + + require.NoError(t, err) + }) + + t.Run("array reference, element", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let array: [[Int]] = [[1, 2]] + let arrayRef = &array as &[[Int]] + var x: &[Int] = arrayRef[0] + } + `) + + require.NoError(t, err) + }) + + t.Run("array reference, optional typed element", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let array: [[Int]?] = [[1, 2]] + let arrayRef = &array as &[[Int]?] + var x: &[Int]? = arrayRef[0] + } + `) + + require.NoError(t, err) + }) + + t.Run("array reference, primitive typed element", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let array: [Int] = [1, 2] + let arrayRef = &array as &[Int] + var x: Int = arrayRef[0] + } + `) + + require.NoError(t, err) + }) + + t.Run("dictionary, value", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let dict: {String: {String: Int}} = {"one": {"two": 2}} + var x: {String: Int}? = dict["one"] + } + `) + + require.NoError(t, err) + }) + + t.Run("dictionary reference, value", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let dict: {String: {String: Int} } = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}} + var x: &{String: Int}? = dictRef["one"] + } + `) + + require.NoError(t, err) + }) + + t.Run("dictionary reference, optional typed value", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let dict: {String: {String: Int}?} = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}?} + var x: (&{String: Int})?? = dictRef["one"] + } + `) + + require.NoError(t, err) + }) + + t.Run("dictionary reference, optional typed value, mismatch types", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let dict: {String: {String: Int}?} = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}?} + + // Must return an optional reference, not a reference to an optional + var x: &({String: Int}??) = dictRef["one"] + } + `) + + errors := RequireCheckerErrors(t, err, 1) + typeMismatchError := &sema.TypeMismatchError{} + require.ErrorAs(t, errors[0], &typeMismatchError) + }) + + t.Run("dictionary reference, primitive typed value", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + fun test() { + let dict: {String: Int} = {"one": 1} + let dictRef = &dict as &{String: Int} + var x: Int? = dictRef["one"] + } + `) + + require.NoError(t, err) + }) + + t.Run("resource reference, attachment", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + resource R {} + + attachment A for R {} + + fun test() { + let r <- create R() + let rRef = &r as &R + + var a: &A? = rRef[A] + destroy r + } + `) + + require.NoError(t, err) + }) +} From ece3d8f64886bd292d2a8ed960a6f0a73552bbd8 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 9 Jun 2023 15:22:44 -0700 Subject: [PATCH 02/24] Fix attachment 'self' access --- runtime/sema/check_expression.go | 3 +-- runtime/sema/check_member_expression.go | 25 ++++++++++++++++++++----- 2 files changed, 21 insertions(+), 7 deletions(-) diff --git a/runtime/sema/check_expression.go b/runtime/sema/check_expression.go index 77996b290c..70ae634921 100644 --- a/runtime/sema/check_expression.go +++ b/runtime/sema/check_expression.go @@ -270,8 +270,7 @@ func (checker *Checker) VisitIndexExpression(expression *ast.IndexExpression) Ty // 1) is accessed via a reference, and // 2) is container-typed, // then the element type should also be a reference. - if _, accessedViaReference := parentType.(*ReferenceType); accessedViaReference && - checker.isContainerType(elementType) { + if shouldReturnReference(parentType, elementType) { elementType = checker.getReferenceType(elementType) } diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index b9bedff77c..f4e6962c8c 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -95,9 +95,16 @@ func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) // 1) is accessed via a reference, and // 2) is container-typed, // then the member type should also be a reference. - if _, accessedViaReference := accessedType.(*ReferenceType); accessedViaReference && - member.DeclarationKind == common.DeclarationKindField && - checker.isContainerType(memberType) { + + // Note: For attachments, `self` is always a reference. + // But we do not want to return a reference for `self.something`. + // Otherwise, things like `destroy self.something` would become invalid. + // Hence, special case `self`, and return a reference only if the member is not accessed via self. + // i.e: `accessedSelfMember == nil` + + if accessedSelfMember == nil && + shouldReturnReference(accessedType, memberType) && + member.DeclarationKind == common.DeclarationKindField { // Get a reference to the type memberType = checker.getReferenceType(memberType) } @@ -120,14 +127,22 @@ func (checker *Checker) getReferenceType(typ Type) Type { return NewReferenceType(checker.memoryGauge, typ, false) } -func (checker *Checker) isContainerType(typ Type) bool { +func shouldReturnReference(parentType, memberType Type) bool { + if _, parentIsReference := parentType.(*ReferenceType); !parentIsReference { + return false + } + + return isContainerType(memberType) +} + +func isContainerType(typ Type) bool { switch typ := typ.(type) { case *CompositeType, *DictionaryType, ArrayType: return true case *OptionalType: - return checker.isContainerType(typ.Type) + return isContainerType(typ.Type) default: return false } From ccc053ea54001cb895e2e769b306acab9407a4e7 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 12 Jun 2023 10:04:52 -0700 Subject: [PATCH 03/24] Update interpreter member access --- runtime/interpreter/interpreter_expression.go | 87 +++++- runtime/tests/interpreter/member_test.go | 270 ++++++++++++++++++ runtime/tests/interpreter/reference_test.go | 10 +- runtime/tests/interpreter/resources_test.go | 2 +- 4 files changed, 361 insertions(+), 8 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 210a5ec37b..34b04751fb 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -144,7 +144,10 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression if isNestedResourceMove { return target.RemoveKey(interpreter, locationRange, transferredIndexingValue) } else { - return target.GetKey(interpreter, locationRange, transferredIndexingValue) + value := target.GetKey(interpreter, locationRange, transferredIndexingValue) + + // If the indexing value is a reference, then return a reference for the resulting value. + return interpreter.maybeGetReference(indexExpression, target, value) } }, set: func(value Value) { @@ -169,6 +172,12 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a isNestedResourceMove := interpreter.Program.Elaboration.IsNestedResourceMoveExpression(memberExpression) + memberInfo, ok := interpreter.Program.Elaboration.MemberExpressionMemberInfo(memberExpression) + if !ok { + panic(errors.NewUnreachableError()) + } + memberType := memberInfo.Member.TypeAnnotation.Type + return getterSetter{ target: target, get: func(allowMissing bool) Value { @@ -212,6 +221,16 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } } + accessingSelf := false + if identifierExpression, ok := memberExpression.Expression.(*ast.IdentifierExpression); ok { + accessingSelf = identifierExpression.Identifier.Identifier == sema.SelfIdentifier + } + + if !accessingSelf && shouldReturnReference(target, resultValue) { + // Get a reference to the value + resultValue = interpreter.getReferenceValue(resultValue, memberType) + } + return resultValue }, set: func(value Value) { @@ -222,6 +241,51 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } } +func shouldReturnReference(parent, member Value) bool { + if _, parentIsReference := parent.(ReferenceValue); !parentIsReference { + return false + } + + return isContainerValue(member) +} + +func isContainerValue(value Value) bool { + switch value := value.(type) { + case *CompositeValue, + *SimpleCompositeValue, + *DictionaryValue, + *ArrayValue: + return true + case *SomeValue: + return isContainerValue(value.value) + default: + return false + } +} + +// getReferenceType Returns a reference type to a given type. +// Reference to an optional should return an optional reference. +// This has to be done recursively for nested optionals. +// e.g.1: Given type T, this method returns &T. +// e.g.2: Given T?, this returns (&T)? +func (interpreter *Interpreter) getReferenceValue(value Value, semaType sema.Type) Value { + if optionalValue, ok := value.(*SomeValue); ok { + optionalType, ok := semaType.(*sema.OptionalType) + if !ok { + // If the value is optional, type must also be optional + // TODO: Is this always true? + panic(errors.NewUnreachableError()) + } + semaType = optionalType.Type + + innerValue := interpreter.getReferenceValue(optionalValue.value, semaType) + return NewSomeValueNonCopying(interpreter, innerValue) + } + + interpreter.maybeTrackReferencedResourceKindedValue(value) + return NewEphemeralReferenceValue(interpreter, false, value, semaType) +} + func (interpreter *Interpreter) checkMemberAccess( memberExpression *ast.MemberExpression, target Value, @@ -857,10 +921,29 @@ func (interpreter *Interpreter) VisitIndexExpression(expression *ast.IndexExpres Location: interpreter.Location, HasPosition: expression, } - return typedResult.GetKey(interpreter, locationRange, indexingValue) + value := typedResult.GetKey(interpreter, locationRange, indexingValue) + + // If the indexing value is a reference, then return a reference for the resulting value. + return interpreter.maybeGetReference(expression, typedResult, value) } } +func (interpreter *Interpreter) maybeGetReference( + expression *ast.IndexExpression, + parentValue ValueIndexableValue, + memberValue Value, +) Value { + if shouldReturnReference(parentValue, memberValue) { + indexExpressionTypes := interpreter.Program.Elaboration.IndexExpressionTypes(expression) + elementType := indexExpressionTypes.IndexedType.ElementType(false) + + // Get a reference to the value + memberValue = interpreter.getReferenceValue(memberValue, elementType) + } + + return memberValue +} + func (interpreter *Interpreter) VisitConditionalExpression(expression *ast.ConditionalExpression) Value { value, ok := interpreter.evalExpression(expression.Test).(BoolValue) if !ok { diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 7a3c03e269..114dd1a76b 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -557,3 +557,273 @@ func TestInterpretMemberAccessType(t *testing.T) { }) }) } + +func TestInterpretMemberAccess(t *testing.T) { + + t.Parallel() + + t.Run("composite, field", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + var x: [Int] + init() { + self.x = [] + } + } + + fun test(): [Int] { + let test = Test() + return test.x + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("composite, function", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + pub fun foo(): Int { + return 1 + } + } + + fun test() { + let test = Test() + var foo: (fun(): Int) = test.foo + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("composite reference, field", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + var x: [Int] + init() { + self.x = [] + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: &[Int] = testRef.x + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("composite reference, optional field", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + var x: [Int]? + init() { + self.x = [] + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: &[Int]? = testRef.x + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("composite reference, primitive field", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + var x: Int + init() { + self.x = 1 + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var x: Int = testRef.x + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("composite reference, function", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + pub fun foo(): Int { + return 1 + } + } + + fun test() { + let test = Test() + let testRef = &test as &Test + var foo: (fun(): Int) = testRef.foo + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("array, element", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let array: [[Int]] = [[1, 2]] + var x: [Int] = array[0] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("array reference, element", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let array: [[Int]] = [[1, 2]] + let arrayRef = &array as &[[Int]] + var x: &[Int] = arrayRef[0] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("array reference, element, in assignment", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let array: [[Int]] = [[1, 2]] + let arrayRef = &array as &[[Int]] + var x: &[Int] = &[] as &[Int] + x = arrayRef[0] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("array reference, optional typed element", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let array: [[Int]?] = [[1, 2]] + let arrayRef = &array as &[[Int]?] + var x: &[Int]? = arrayRef[0] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("array reference, primitive typed element", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let array: [Int] = [1, 2] + let arrayRef = &array as &[Int] + var x: Int = arrayRef[0] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("dictionary, value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let dict: {String: {String: Int}} = {"one": {"two": 2}} + var x: {String: Int}? = dict["one"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("dictionary reference, value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let dict: {String: {String: Int} } = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}} + var x: &{String: Int}? = dictRef["one"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("dictionary reference, value, in assignment", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let dict: {String: {String: Int} } = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}} + var x: &{String: Int}? = &{} as &{String: Int} + x = dictRef["one"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("dictionary reference, optional typed value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let dict: {String: {String: Int}?} = {"one": {"two": 2}} + let dictRef = &dict as &{String: {String: Int}?} + var x: (&{String: Int})?? = dictRef["one"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("dictionary reference, primitive typed value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test() { + let dict: {String: Int} = {"one": 1} + let dictRef = &dict as &{String: Int} + var x: Int? = dictRef["one"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + + t.Run("resource reference, attachment", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + resource R {} + + attachment A for R {} + + fun test() { + let r <- create R() + let rRef = &r as &R + + var a: &A? = rRef[A] + destroy r + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) +} diff --git a/runtime/tests/interpreter/reference_test.go b/runtime/tests/interpreter/reference_test.go index f13ab2f01b..c7c1d676b2 100644 --- a/runtime/tests/interpreter/reference_test.go +++ b/runtime/tests/interpreter/reference_test.go @@ -645,7 +645,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { target.append(<- create R()) // Take reference while in the account - let ref = &target[0] as &R + let ref = target[0] // Move the resource out of the account onto the stack let movedR <- target.remove(at: 0) @@ -739,7 +739,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { target1.append(<- create R()) // Take reference while in the account_1 - let ref = &target1[0] as &R + let ref = target1[0] // Move the resource out of the account_1 into the account_2 target2.append(<- target1.remove(at: 0)) @@ -811,7 +811,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { target.append(<- create R()) // Take reference while in the account - let ref = &target[0] as &R + let ref = target[0] // Move the resource out of the account onto the stack. This should invalidate the reference. let movedR <- target.remove(at: 0) @@ -915,7 +915,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { collection.append(<- create R()) // Take reference while in the account - ref1 = &collection[0] as &R + ref1 = collection[0] // Move the resource out of the account onto the stack. This should invalidate ref1. let movedR <- collection.remove(at: 0) @@ -930,7 +930,7 @@ func TestInterpretResourceReferenceInvalidationOnMove(t *testing.T) { collection.append(<- movedR) // Take another reference - ref3 = &collection[1] as &R + ref3 = collection[1] } fun getRef1Id(): Int { diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 544a24ea51..6ba4da2970 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2376,7 +2376,7 @@ func TestInterpretArrayOptionalResourceReference(t *testing.T) { account.save(<-[<-create R()], to: /storage/x) let collection = account.borrow<&[R?]>(from: /storage/x)! - let resourceRef = (&collection[0] as &R?)! + let resourceRef = collection[0]! let token <- collection.remove(at: 0) let x = resourceRef.id From e26ff54fce672f0e0e08ecb6d69c782f46afaafc Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 12 Jun 2023 18:15:05 -0700 Subject: [PATCH 04/24] Update tests --- runtime/interpreter/interpreter_expression.go | 21 +- runtime/resourcedictionary_test.go | 6 +- runtime/runtime_test.go | 28 +- .../tests/checker/external_mutation_test.go | 2 +- runtime/tests/checker/reference_test.go | 39 ++- runtime/tests/interpreter/resources_test.go | 273 ------------------ 6 files changed, 51 insertions(+), 318 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 34b04751fb..006c911c06 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -269,16 +269,21 @@ func isContainerValue(value Value) bool { // e.g.1: Given type T, this method returns &T. // e.g.2: Given T?, this returns (&T)? func (interpreter *Interpreter) getReferenceValue(value Value, semaType sema.Type) Value { - if optionalValue, ok := value.(*SomeValue); ok { - optionalType, ok := semaType.(*sema.OptionalType) - if !ok { - // If the value is optional, type must also be optional - // TODO: Is this always true? - panic(errors.NewUnreachableError()) - } + optionalType, ok := semaType.(*sema.OptionalType) + if ok { semaType = optionalType.Type - innerValue := interpreter.getReferenceValue(optionalValue.value, semaType) + // Because the boxing happens further down the code, it is possible + // to have a concrete value (non-some) for a place where optional is expected. + // Therefore, always unwrap the type, but only unwrap if the value is `SomeValue`. + // + // However, checker guarantees that the wise-versa doesn't happen. + // i.e: There will never be a `SomeValue`, with type being a non-optional. + + if optionalValue, ok := value.(*SomeValue); ok { + value = optionalValue.value + } + innerValue := interpreter.getReferenceValue(value, semaType) return NewSomeValueNonCopying(interpreter, innerValue) } diff --git a/runtime/resourcedictionary_test.go b/runtime/resourcedictionary_test.go index e43d231db1..035d3e10cb 100644 --- a/runtime/resourcedictionary_test.go +++ b/runtime/resourcedictionary_test.go @@ -248,11 +248,12 @@ func TestRuntimeResourceDictionaryValues(t *testing.T) { transaction { prepare(signer: AuthAccount) { - let c = signer.borrow<&Test.C>(from: /storage/c)! + let c <- signer.load<@Test.C>(from: /storage/c)! log(c.rs["b"]?.value) let existing <- c.rs["b"] <- Test.createR(4) destroy existing log(c.rs["b"]?.value) + signer.save(<-c, to: /storage/c) } } `) @@ -288,11 +289,12 @@ func TestRuntimeResourceDictionaryValues(t *testing.T) { transaction { prepare(signer: AuthAccount) { - let c = signer.borrow<&Test.C>(from: /storage/c)! + let c <- signer.load<@Test.C>(from: /storage/c)! log(c.rs["b"]?.value) let existing <- c.rs["b"] <- nil destroy existing log(c.rs["b"]?.value) + signer.save(<-c, to: /storage/c) } } `) diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 37aa5208ac..dac0dc9bc4 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -5324,26 +5324,26 @@ func TestRuntimeResourceOwnerFieldUseDictionary(t *testing.T) { "a": <-Test.createR(), "b": <-Test.createR() } - log(rs["a"]?.owner?.address) - log(rs["b"]?.owner?.address) - rs["a"]?.logOwnerAddress() - rs["b"]?.logOwnerAddress() + //log(rs["a"]?.owner?.address) + //log(rs["b"]?.owner?.address) + //rs["a"]?.logOwnerAddress() + //rs["b"]?.logOwnerAddress() signer.save(<-rs, to: /storage/rs) signer.link<&{String: Test.R}>(/public/rs, target: /storage/rs) let ref1 = signer.borrow<&{String: Test.R}>(from: /storage/rs)! log(ref1["a"]?.owner?.address) - log(ref1["b"]?.owner?.address) - ref1["a"]?.logOwnerAddress() - ref1["b"]?.logOwnerAddress() - - let publicAccount = getAccount(0x01) - let ref2 = publicAccount.getCapability(/public/rs).borrow<&{String: Test.R}>()! - log(ref2["a"]?.owner?.address) - log(ref2["b"]?.owner?.address) - ref2["a"]?.logOwnerAddress() - ref2["b"]?.logOwnerAddress() + //log(ref1["b"]?.owner?.address) + //ref1["a"]?.logOwnerAddress() + //ref1["b"]?.logOwnerAddress() + // + //let publicAccount = getAccount(0x01) + //let ref2 = publicAccount.getCapability(/public/rs).borrow<&{String: Test.R}>()! + //log(ref2["a"]?.owner?.address) + //log(ref2["b"]?.owner?.address) + //ref2["a"]?.logOwnerAddress() + //ref2["b"]?.logOwnerAddress() } } `) diff --git a/runtime/tests/checker/external_mutation_test.go b/runtime/tests/checker/external_mutation_test.go index d2428afbff..da2007ea80 100644 --- a/runtime/tests/checker/external_mutation_test.go +++ b/runtime/tests/checker/external_mutation_test.go @@ -799,7 +799,7 @@ func TestCheckMutationThroughInnerReference(t *testing.T) { ` pub fun main() { let foo = Foo() - var arrayRef = &foo.ref.arr as &[String] + var arrayRef = foo.ref.arr arrayRef[0] = "y" } diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go index 030597c602..122b9e0e46 100644 --- a/runtime/tests/checker/reference_test.go +++ b/runtime/tests/checker/reference_test.go @@ -784,10 +784,8 @@ func TestCheckReferenceIndexingIfReferencedIndexable(t *testing.T) { fun test() { let rs <- [<-create R()] let ref = &rs as &[R] - var other <- create R() - ref[0] <-> other + ref[0] destroy rs - destroy other } `) @@ -805,8 +803,7 @@ func TestCheckReferenceIndexingIfReferencedIndexable(t *testing.T) { fun test() { let s = [S()] let ref = &s as &[S] - var other = S() - ref[0] <-> other + ref[0] } `) @@ -814,7 +811,7 @@ func TestCheckReferenceIndexingIfReferencedIndexable(t *testing.T) { }) } -func TestCheckInvalidReferenceResourceLoss(t *testing.T) { +func TestCheckReferenceResourceLoss(t *testing.T) { t.Parallel() @@ -824,17 +821,15 @@ func TestCheckInvalidReferenceResourceLoss(t *testing.T) { fun test() { let rs <- [<-create R()] let ref = &rs as &[R] - ref[0] + ref[0] // This result in a reference, so no resource loss destroy rs } `) - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.ResourceLossError{}, errs[0]) + require.NoError(t, err) } -func TestCheckInvalidReferenceResourceLoss2(t *testing.T) { +func TestCheckInvalidReferenceResourceLoss(t *testing.T) { t.Parallel() @@ -1705,7 +1700,7 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { authAccount.save(<-[<-create R()], to: /storage/a) let collectionRef = authAccount.borrow<&[R]>(from: /storage/a)! - let ref = &collectionRef[0] as &R + let ref = collectionRef[0] let collection <- authAccount.load<@[R]>(from: /storage/a)! authAccount.save(<- collection, to: /storage/b) @@ -1829,8 +1824,8 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { pub fun test() { var r: @{UInt64: {UInt64: [R]}} <- {} let ref1 = (&r[0] as &{UInt64: [R]}?)! - let ref2 = (&ref1[0] as &[R]?)! - let ref3 = &ref2[0] as &R + let ref2 = ref1[0]! + let ref3 = ref2[0] ref3.a destroy r @@ -1858,8 +1853,8 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { pub fun test() { var r: @{UInt64: {UInt64: [R]}} <- {} let ref1 = (&r[0] as &{UInt64: [R]}?)! - let ref2 = (&ref1[0] as &[R]?)! - let ref3 = &ref2[0] as &R + let ref2 = ref1[0]! + let ref3 = ref2[0] destroy r ref3.a } @@ -1996,7 +1991,7 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { pub resource R { pub fun test() { if let storage = &Test.a[0] as &{UInt64: Test.R}? { - let nftRef = (&storage[0] as &Test.R?)! + let nftRef = storage[0]! nftRef } } @@ -2053,9 +2048,9 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { pub contract Test { pub resource R { pub fun test(packList: &[Test.R]) { - var i = 0; + var i = 0 while i < packList.length { - let pack = &packList[i] as &Test.R; + let pack = packList[i] pack i = i + 1 } @@ -2668,10 +2663,14 @@ func TestCheckReferenceUseAfterCopy(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 2) + errs := RequireCheckerErrors(t, err, 3) + invalidatedRefError := &sema.InvalidatedResourceReferenceError{} assert.ErrorAs(t, errs[0], &invalidatedRefError) assert.ErrorAs(t, errs[1], &invalidatedRefError) + + typeMismatchError := &sema.TypeMismatchError{} + assert.ErrorAs(t, errs[2], &typeMismatchError) }) t.Run("resource array, remove", func(t *testing.T) { diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 6ba4da2970..9c6d346bbf 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -24,8 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/onflow/atree" - "github.com/onflow/cadence/runtime/sema" "github.com/onflow/cadence/runtime/tests/checker" . "github.com/onflow/cadence/runtime/tests/utils" @@ -145,73 +143,6 @@ func TestInterpretImplicitResourceRemovalFromContainer(t *testing.T) { ) }) - t.Run("reference, shift statement, member expression", func(t *testing.T) { - - t.Parallel() - - inter := parseCheckAndInterpret(t, ` - resource R2 { - let value: String - - init() { - self.value = "test" - } - } - - resource R1 { - var r2: @R2? - - init() { - self.r2 <- nil - } - - destroy() { - destroy self.r2 - } - } - - fun createR1(): @R1 { - return <- create R1() - } - - fun test(r1: &R1): String? { - r1.r2 <-! create R2() - // The second assignment should not lead to the resource being cleared, - // it must be fully moved out of this container before, - // not just assigned to the new variable - let optR2 <- r1.r2 <- nil - let value = optR2?.value - destroy optR2 - return value - } - `) - - r1, err := inter.Invoke("createR1") - require.NoError(t, err) - - r1 = r1.Transfer(inter, interpreter.EmptyLocationRange, atree.Address{1}, false, nil) - - r1Type := checker.RequireGlobalType(t, inter.Program.Elaboration, "R1") - - ref := interpreter.NewUnmeteredEphemeralReferenceValue( - false, - r1, - r1Type, - ) - - value, err := inter.Invoke("test", ref) - require.NoError(t, err) - - AssertValuesEqual( - t, - inter, - interpreter.NewUnmeteredSomeValueNonCopying( - interpreter.NewUnmeteredStringValue("test"), - ), - value, - ) - }) - t.Run("resource, if-let statement, member expression", func(t *testing.T) { t.Parallel() @@ -266,75 +197,6 @@ func TestInterpretImplicitResourceRemovalFromContainer(t *testing.T) { ) }) - t.Run("reference, if-let statement, member expression", func(t *testing.T) { - - t.Parallel() - - inter := parseCheckAndInterpret(t, ` - resource R2 { - let value: String - - init() { - self.value = "test" - } - } - - resource R1 { - var r2: @R2? - - init() { - self.r2 <- nil - } - - destroy() { - destroy self.r2 - } - } - - fun createR1(): @R1 { - return <- create R1() - } - - fun test(r1: &R1): String? { - r1.r2 <-! create R2() - // The second assignment should not lead to the resource being cleared, - // it must be fully moved out of this container before, - // not just assigned to the new variable - if let r2 <- r1.r2 <- nil { - let value = r2.value - destroy r2 - return value - } - return nil - } - `) - - r1, err := inter.Invoke("createR1") - require.NoError(t, err) - - r1 = r1.Transfer(inter, interpreter.EmptyLocationRange, atree.Address{1}, false, nil) - - r1Type := checker.RequireGlobalType(t, inter.Program.Elaboration, "R1") - - ref := interpreter.NewUnmeteredEphemeralReferenceValue( - false, - r1, - r1Type, - ) - - value, err := inter.Invoke("test", ref) - require.NoError(t, err) - - AssertValuesEqual( - t, - inter, - interpreter.NewUnmeteredSomeValueNonCopying( - interpreter.NewUnmeteredStringValue("test"), - ), - value, - ) - }) - t.Run("resource, shift statement, index expression", func(t *testing.T) { t.Parallel() @@ -386,72 +248,6 @@ func TestInterpretImplicitResourceRemovalFromContainer(t *testing.T) { ) }) - t.Run("reference, shift statement, index expression", func(t *testing.T) { - - t.Parallel() - - inter := parseCheckAndInterpret(t, ` - resource R2 { - let value: String - - init() { - self.value = "test" - } - } - - resource R1 { - pub(set) var r2s: @{Int: R2} - - init() { - self.r2s <- {} - } - - destroy() { - destroy self.r2s - } - } - - fun createR1(): @R1 { - return <- create R1() - } - - fun test(r1: &R1): String? { - r1.r2s[0] <-! create R2() - // The second assignment should not lead to the resource being cleared, - // it must be fully moved out of this container before, - // not just assigned to the new variable - let optR2 <- r1.r2s[0] <- nil - let value = optR2?.value - destroy optR2 - return value - } - `) - - r1, err := inter.Invoke("createR1") - require.NoError(t, err) - - r1 = r1.Transfer(inter, interpreter.EmptyLocationRange, atree.Address{1}, false, nil) - - r1Type := checker.RequireGlobalType(t, inter.Program.Elaboration, "R1") - - ref := interpreter.NewUnmeteredEphemeralReferenceValue( - false, - r1, - r1Type, - ) - value, err := inter.Invoke("test", ref) - require.NoError(t, err) - - AssertValuesEqual( - t, - inter, - interpreter.NewUnmeteredSomeValueNonCopying( - interpreter.NewUnmeteredStringValue("test"), - ), - value, - ) - }) - t.Run("resource, if-let statement, index expression", func(t *testing.T) { t.Parallel() @@ -505,75 +301,6 @@ func TestInterpretImplicitResourceRemovalFromContainer(t *testing.T) { value, ) }) - - t.Run("reference, if-let statement, index expression", func(t *testing.T) { - - t.Parallel() - - inter := parseCheckAndInterpret(t, ` - resource R2 { - let value: String - - init() { - self.value = "test" - } - } - - resource R1 { - var r2s: @{Int: R2} - - init() { - self.r2s <- {} - } - - destroy() { - destroy self.r2s - } - } - - fun createR1(): @R1 { - return <- create R1() - } - - fun test(r1: &R1): String? { - r1.r2s[0] <-! create R2() - // The second assignment should not lead to the resource being cleared, - // it must be fully moved out of this container before, - // not just assigned to the new variable - if let r2 <- r1.r2s[0] <- nil { - let value = r2.value - destroy r2 - return value - } - return nil - } - `) - - r1, err := inter.Invoke("createR1") - require.NoError(t, err) - - r1 = r1.Transfer(inter, interpreter.EmptyLocationRange, atree.Address{1}, false, nil) - - r1Type := checker.RequireGlobalType(t, inter.Program.Elaboration, "R1") - - ref := interpreter.NewUnmeteredEphemeralReferenceValue( - false, - r1, - r1Type, - ) - - value, err := inter.Invoke("test", ref) - require.NoError(t, err) - - AssertValuesEqual( - t, - inter, - interpreter.NewUnmeteredSomeValueNonCopying( - interpreter.NewUnmeteredStringValue("test"), - ), - value, - ) - }) } func TestInterpretInvalidatedResourceValidation(t *testing.T) { From ab854e2fe5377dcb3e8c4370c85c224d48435b9c Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 13 Jun 2023 14:44:44 -0700 Subject: [PATCH 05/24] Fix more tests --- runtime/missingmember_test.go | 42 +++++++++++---------- runtime/runtime_test.go | 28 +++++++------- runtime/sema/check_member_expression.go | 3 +- runtime/storage_test.go | 8 ++-- runtime/tests/interpreter/resources_test.go | 2 +- 5 files changed, 43 insertions(+), 40 deletions(-) diff --git a/runtime/missingmember_test.go b/runtime/missingmember_test.go index b1e427881e..e4cce8b453 100644 --- a/runtime/missingmember_test.go +++ b/runtime/missingmember_test.go @@ -3542,18 +3542,18 @@ pub contract AuctionDutch { pub let currentPrice: UFix64 pub let totalItems: Int pub let acceptedBids: Int - pub let tickStatus: {UFix64:TickStatus} - pub let metadata: {String:String} - - init(status:String, currentPrice: UFix64, totalItems: Int, acceptedBids:Int, startTime: UFix64, tickStatus: {UFix64:TickStatus}, metadata: {String:String}){ - self.status=status - self.currentPrice=currentPrice - self.totalItems=totalItems - self.acceptedBids=acceptedBids - self.startTime=startTime - self.currentTime= 42.0 // Clock.time() - self.tickStatus=tickStatus - self.metadata=metadata + pub let tickStatus: &{UFix64:TickStatus} + pub let metadata: &{String:String} + + init(status:String, currentPrice: UFix64, totalItems: Int, acceptedBids:Int, startTime: UFix64, tickStatus: &{UFix64:TickStatus}, metadata: &{String:String}){ + self.status = status + self.currentPrice = currentPrice + self.totalItems = totalItems + self.acceptedBids = acceptedBids + self.startTime = startTime + self.currentTime = 42.0 // Clock.time() + self.tickStatus = tickStatus + self.metadata = metadata } } @@ -3573,7 +3573,7 @@ pub contract AuctionDutch { } pub fun getStatus(_ id: UInt64) : AuctionDutchStatus{ - let item= self.getAuction(id) + let item = self.getAuction(id) let currentTime= 42.0 // Clock.time() var status="Ongoing" @@ -3586,13 +3586,15 @@ pub contract AuctionDutch { } - return AuctionDutchStatus(status: status, - currentPrice: currentPrice, - totalItems: item.numberOfItems, - acceptedBids: item.winningBids.length, - startTime: item.startAt(), - tickStatus: item.auctionStatus, - metadata:item.metadata) + return AuctionDutchStatus( + status: status, + currentPrice: currentPrice, + totalItems: item.numberOfItems, + acceptedBids: item.winningBids.length, + startTime: item.startAt(), + tickStatus: item.auctionStatus, + metadata:item.metadata, + ) } pub fun getBids(_ id:UInt64) : Bids { diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index dac0dc9bc4..37aa5208ac 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -5324,26 +5324,26 @@ func TestRuntimeResourceOwnerFieldUseDictionary(t *testing.T) { "a": <-Test.createR(), "b": <-Test.createR() } - //log(rs["a"]?.owner?.address) - //log(rs["b"]?.owner?.address) - //rs["a"]?.logOwnerAddress() - //rs["b"]?.logOwnerAddress() + log(rs["a"]?.owner?.address) + log(rs["b"]?.owner?.address) + rs["a"]?.logOwnerAddress() + rs["b"]?.logOwnerAddress() signer.save(<-rs, to: /storage/rs) signer.link<&{String: Test.R}>(/public/rs, target: /storage/rs) let ref1 = signer.borrow<&{String: Test.R}>(from: /storage/rs)! log(ref1["a"]?.owner?.address) - //log(ref1["b"]?.owner?.address) - //ref1["a"]?.logOwnerAddress() - //ref1["b"]?.logOwnerAddress() - // - //let publicAccount = getAccount(0x01) - //let ref2 = publicAccount.getCapability(/public/rs).borrow<&{String: Test.R}>()! - //log(ref2["a"]?.owner?.address) - //log(ref2["b"]?.owner?.address) - //ref2["a"]?.logOwnerAddress() - //ref2["b"]?.logOwnerAddress() + log(ref1["b"]?.owner?.address) + ref1["a"]?.logOwnerAddress() + ref1["b"]?.logOwnerAddress() + + let publicAccount = getAccount(0x01) + let ref2 = publicAccount.getCapability(/public/rs).borrow<&{String: Test.R}>()! + log(ref2["a"]?.owner?.address) + log(ref2["b"]?.owner?.address) + ref2["a"]?.logOwnerAddress() + ref2["b"]?.logOwnerAddress() } } `) diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index f4e6962c8c..923f4a212b 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -128,7 +128,8 @@ func (checker *Checker) getReferenceType(typ Type) Type { } func shouldReturnReference(parentType, memberType Type) bool { - if _, parentIsReference := parentType.(*ReferenceType); !parentIsReference { + unwrappedParentType := UnwrapOptionalType(parentType) + if _, parentIsReference := unwrappedParentType.(*ReferenceType); !parentIsReference { return false } diff --git a/runtime/storage_test.go b/runtime/storage_test.go index 0758419b55..e1a54004a9 100644 --- a/runtime/storage_test.go +++ b/runtime/storage_test.go @@ -2422,7 +2422,7 @@ func TestRuntimeReferenceOwnerAccess(t *testing.T) { // At this point the resource is in storage account.link<&[TestContract.TestResource]>(/public/test, target: /storage/test) let ref2 = account.getCapability<&[TestContract.TestResource]>(/public/test).borrow()! - let ref3 = &ref2[0] as &TestContract.TestResource + let ref3 = ref2[0] log(ref3.owner?.address) } } @@ -2558,7 +2558,7 @@ func TestRuntimeReferenceOwnerAccess(t *testing.T) { // At this point the nesting and nested resources are both in storage account.link<&TestContract.TestNestingResource>(/public/test, target: /storage/test) nestingResourceRef = account.getCapability<&TestContract.TestNestingResource>(/public/test).borrow()! - nestedElementResourceRef = &nestingResourceRef.nestedResources[0] as &TestContract.TestNestedResource + nestedElementResourceRef = nestingResourceRef.nestedResources[0] log(nestingResourceRef.owner?.address) log(nestedElementResourceRef.owner?.address) @@ -2684,7 +2684,7 @@ func TestRuntimeReferenceOwnerAccess(t *testing.T) { // At this point the resource is in storage account.link<&[[TestContract.TestResource]]>(/public/test, target: /storage/test) let testResourcesRef = account.getCapability<&[[TestContract.TestResource]]>(/public/test).borrow()! - ref = &testResourcesRef[0] as &[TestContract.TestResource] + ref = testResourcesRef[0] log(ref[0].owner?.address) } } @@ -2806,7 +2806,7 @@ func TestRuntimeReferenceOwnerAccess(t *testing.T) { // At this point the resource is in storage account.link<&[{Int: TestContract.TestResource}]>(/public/test, target: /storage/test) let testResourcesRef = account.getCapability<&[{Int: TestContract.TestResource}]>(/public/test).borrow()! - ref = &testResourcesRef[0] as &{Int: TestContract.TestResource} + ref = testResourcesRef[0] log(ref[0]?.owner?.address) } } diff --git a/runtime/tests/interpreter/resources_test.go b/runtime/tests/interpreter/resources_test.go index 9c6d346bbf..93a6147cfa 100644 --- a/runtime/tests/interpreter/resources_test.go +++ b/runtime/tests/interpreter/resources_test.go @@ -2065,7 +2065,7 @@ func TestInterpretOptionalResourceReference(t *testing.T) { account.save(<-{0 : <-create R()}, to: /storage/x) let collection = account.borrow<&{Int: R}>(from: /storage/x)! - let resourceRef = (&collection[0] as &R?)! + let resourceRef = collection[0]! let token <- collection.remove(key: 0) let x = resourceRef.id From 85d294a1fd357f4c5e68c53596bdb943f064eb5c Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 08:28:34 -0700 Subject: [PATCH 06/24] Add member-acess expr to static reference validity checking --- runtime/attachments_test.go | 10 +++- runtime/sema/check_member_expression.go | 9 +++- runtime/sema/check_variable_declaration.go | 18 ++++++- runtime/tests/checker/reference_test.go | 51 ++++++++++++++++++- runtime/tests/interpreter/attachments_test.go | 41 ++++++++++----- 5 files changed, 110 insertions(+), 19 deletions(-) diff --git a/runtime/attachments_test.go b/runtime/attachments_test.go index daa1bff141..f77cadc816 100644 --- a/runtime/attachments_test.go +++ b/runtime/attachments_test.go @@ -170,10 +170,18 @@ func TestAccountAttachmentExportFailure(t *testing.T) { import Test from 0x1 pub fun main(): &Test.A? { let r <- Test.makeRWithA() - let a = r[Test.A] + var a = r[Test.A] + + // just to trick the checker + a = returnSameRef(a) + destroy r return a } + + pub fun returnSameRef(_ ref: &Test.A?): &Test.A? { + return ref + } `) runtimeInterface1 := &testRuntimeInterface{ diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 923f4a212b..2766454736 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -128,14 +128,19 @@ func (checker *Checker) getReferenceType(typ Type) Type { } func shouldReturnReference(parentType, memberType Type) bool { - unwrappedParentType := UnwrapOptionalType(parentType) - if _, parentIsReference := unwrappedParentType.(*ReferenceType); !parentIsReference { + if !isReferenceType(parentType) { return false } return isContainerType(memberType) } +func isReferenceType(typ Type) bool { + unwrappedType := UnwrapOptionalType(typ) + _, isReference := unwrappedType.(*ReferenceType) + return isReference +} + func isContainerType(typ Type) bool { switch typ := typ.(type) { case *CompositeType, diff --git a/runtime/sema/check_variable_declaration.go b/runtime/sema/check_variable_declaration.go index 66a1bf26a0..deca084e91 100644 --- a/runtime/sema/check_variable_declaration.go +++ b/runtime/sema/check_variable_declaration.go @@ -258,7 +258,7 @@ func (checker *Checker) recordReferenceCreation(target, expr ast.Expression) { } func (checker *Checker) recordReference(targetVariable *Variable, expr ast.Expression) { - if targetVariable == nil { + if targetVariable == nil || !isReferenceType(targetVariable.Type) { return } @@ -280,6 +280,16 @@ func (checker *Checker) referencedVariables(expr ast.Expression) (variables []*V variableRefExpr = rootVariableOfExpression(refExpr.Expression) case *ast.IdentifierExpression: variableRefExpr = &refExpr.Identifier + case *ast.IndexExpression: + // If it is a reference expression, then find the "root variable". + // As nested resources cannot be tracked, at least track the "root" if possible. + // For example, for an expression `a[b][c]`, the "root variable" is `a`. + variableRefExpr = rootVariableOfExpression(refExpr.TargetExpression) + case *ast.MemberExpression: + // If it is a reference expression, then find the "root variable". + // As nested resources cannot be tracked, at least track the "root" if possible. + // For example, for an expression `a.b.c`, the "root variable" is `a`. + variableRefExpr = rootVariableOfExpression(refExpr.Expression) default: continue } @@ -366,7 +376,11 @@ func referenceExpressions(expr ast.Expression) []ast.Expression { } return refExpressions - case *ast.IdentifierExpression: + case *ast.IdentifierExpression, + *ast.IndexExpression, + *ast.MemberExpression: + // For all these expressions, we reach here only if the expression's type is a reference. + // Hence, no need to check it here again. return []ast.Expression{expr} default: return nil diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go index 122b9e0e46..dc9d1cdec6 100644 --- a/runtime/tests/checker/reference_test.go +++ b/runtime/tests/checker/reference_test.go @@ -1844,7 +1844,7 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { require.NoError(t, err) }) - t.Run("ref to ref invalid", func(t *testing.T) { + t.Run("ref to ref invalid, index expr", func(t *testing.T) { t.Parallel() @@ -1873,6 +1873,55 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { assert.ErrorAs(t, errors[0], &invalidatedRefError) }) + t.Run("ref to ref invalid, member expr", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, + ` + pub fun test() { + var r: @R1 <- create R1() + let ref1 = &r as &R1 + let ref2 = ref1.r2 + let ref3 = ref2.r3 + destroy r + ref3.a + } + + pub resource R1 { + pub let r2: @R2 + init() { + self.r2 <- create R2() + } + destroy() { + destroy self.r2 + } + } + + pub resource R2 { + pub let r3: @R3 + init() { + self.r3 <- create R3() + } + destroy() { + destroy self.r3 + } + } + + pub resource R3 { + pub let a: Int + init() { + self.a = 5 + } + } + `, + ) + + errors := RequireCheckerErrors(t, err, 1) + invalidatedRefError := &sema.InvalidatedResourceReferenceError{} + assert.ErrorAs(t, errors[0], &invalidatedRefError) + }) + t.Run("create ref with force expr", func(t *testing.T) { t.Parallel() diff --git a/runtime/tests/interpreter/attachments_test.go b/runtime/tests/interpreter/attachments_test.go index 216afb1ddb..a1101c6a9b 100644 --- a/runtime/tests/interpreter/attachments_test.go +++ b/runtime/tests/interpreter/attachments_test.go @@ -1578,7 +1578,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { fun test(): UInt8 { let r <- create R() let r2 <- attach A() to <-r - let a = r2[A]! + let a = returnSameRef(r2[A]!) // Move the resource after taking a reference to the attachment. @@ -1590,7 +1590,11 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { // Access the attachment filed from the previous reference. return a.id - }`, + } + + pub fun returnSameRef(_ ref: &A): &A { + return ref + }`, sema.Config{ AttachmentsEnabled: true, }, @@ -1614,13 +1618,17 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { fun test() { let r <- create R() let r2 <- attach A() to <-r - let a = r2[A]! + let a = returnSameRef(r2[A]!) destroy r2 let i = a.foo() } - `, sema.Config{ - AttachmentsEnabled: true, - }, + + pub fun returnSameRef(_ ref: &A): &A { + return ref + }`, + sema.Config{ + AttachmentsEnabled: true, + }, ) _, err := inter.Invoke("test") @@ -1652,7 +1660,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { } fun test(): UInt8 { let r2 <- create R2(r: <-attach A() to <-create R()) - let a = r2.r[A]! + let a = returnSameRef(r2.r[A]!) // Move the resource after taking a reference to the attachment. // Then update the field of the attachment. @@ -1663,7 +1671,11 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { // Access the attachment filed from the previous reference. return a.id - }`, + } + + pub fun returnSameRef(_ ref: &A): &A { + return ref + }`, sema.Config{ AttachmentsEnabled: true, }, @@ -1737,14 +1749,17 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { } fun test() { let r2 <- create R2(r: <-attach A() to <-create R()) - let a = r2.r[A]! + let a = returnSameRef(r2.r[A]!) destroy r2 let i = a.foo() } - - `, sema.Config{ - AttachmentsEnabled: true, - }, + + pub fun returnSameRef(_ ref: &A): &A { + return ref + }`, + sema.Config{ + AttachmentsEnabled: true, + }, ) _, err := inter.Invoke("test") From 0421eb0ac3ec56e764c0b0e8e2e2ccc6a92f92b5 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 08:33:37 -0700 Subject: [PATCH 07/24] Add test for static attachment invalidation --- runtime/tests/checker/reference_test.go | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go index dc9d1cdec6..523483e14b 100644 --- a/runtime/tests/checker/reference_test.go +++ b/runtime/tests/checker/reference_test.go @@ -2786,4 +2786,35 @@ func TestCheckReferenceUseAfterCopy(t *testing.T) { invalidatedRefError := &sema.InvalidatedResourceReferenceError{} assert.ErrorAs(t, errs[0], &invalidatedRefError) }) + + t.Run("attachments", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R {} + + attachment A for R { + pub(set) var id: UInt8 + init() { + self.id = 1 + } + } + + fun test() { + let r <- create R() + let r2 <- attach A() to <-r + + let a = r2[A]! + destroy r2 + + // Access attachment ref, after destroying the resource + a.id + } + `) + + errs := RequireCheckerErrors(t, err, 1) + invalidatedRefError := &sema.InvalidatedResourceReferenceError{} + assert.ErrorAs(t, errs[0], &invalidatedRefError) + }) } From b965147772b00e0a09b3769a6ddbe020d8e072b2 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 08:45:20 -0700 Subject: [PATCH 08/24] Refactor code --- runtime/interpreter/interpreter_expression.go | 16 ++++++++++------ runtime/missingmember_test.go | 2 +- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 006c911c06..cbc87cb5f4 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -221,6 +221,13 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } } + // Return a reference, if the member is accessed via a reference. + // + // However, for attachments, `self` is always a reference. + // But we do not want to return a reference for `self.something`. + // Otherwise, things like `destroy self.something` would become invalid. + // Hence, special case `self`, and return a reference only if the member is not accessed via self. + accessingSelf := false if identifierExpression, ok := memberExpression.Expression.(*ast.IdentifierExpression); ok { accessingSelf = identifierExpression.Identifier.Identifier == sema.SelfIdentifier @@ -242,11 +249,8 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } func shouldReturnReference(parent, member Value) bool { - if _, parentIsReference := parent.(ReferenceValue); !parentIsReference { - return false - } - - return isContainerValue(member) + _, parentIsReference := parent.(ReferenceValue) + return parentIsReference && isContainerValue(member) } func isContainerValue(value Value) bool { @@ -263,7 +267,7 @@ func isContainerValue(value Value) bool { } } -// getReferenceType Returns a reference type to a given type. +// getReferenceValue Returns a reference to a given value. // Reference to an optional should return an optional reference. // This has to be done recursively for nested optionals. // e.g.1: Given type T, this method returns &T. diff --git a/runtime/missingmember_test.go b/runtime/missingmember_test.go index e4cce8b453..06612d8b1b 100644 --- a/runtime/missingmember_test.go +++ b/runtime/missingmember_test.go @@ -3587,7 +3587,7 @@ pub contract AuctionDutch { return AuctionDutchStatus( - status: status, + status: status, currentPrice: currentPrice, totalItems: item.numberOfItems, acceptedBids: item.winningBids.length, From aabca06e8c57e47ddbfc9a7d822cb1ce8980b876 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Jun 2023 14:56:33 -0700 Subject: [PATCH 09/24] Add more tests --- runtime/tests/interpreter/member_test.go | 48 ++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 114dd1a76b..e1318f4354 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -678,6 +678,54 @@ func TestInterpretMemberAccess(t *testing.T) { require.NoError(t, err) }) + t.Run("resource reference, nested", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + resource Foo { + var bar: @Bar + init() { + self.bar <- create Bar() + } + destroy() { + destroy self.bar + } + } + + resource Bar { + var baz: @Baz + init() { + self.baz <- create Baz() + } + destroy() { + destroy self.baz + } + } + + resource Baz { + var x: &[Int] + init() { + self.x = &[] as &[Int] + } + } + + fun test() { + let foo <- create Foo() + let fooRef = &foo as &Foo + + // Nested container fields must return references + var barRef: &Bar = fooRef.bar + var bazRef: &Baz = fooRef.bar.baz + + // Reference typed field should return as is (no double reference must be created) + var x: &[Int] = fooRef.bar.baz.x + + destroy foo + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) + t.Run("array, element", func(t *testing.T) { inter := parseCheckAndInterpret(t, ` fun test() { From 04e7ac8ae73ac96834c6c6ebdb677d8ceb9a8504 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 15 Jun 2023 12:32:56 -0700 Subject: [PATCH 10/24] Update to match entitlements --- runtime/interpreter/interpreter_expression.go | 2 +- runtime/sema/check_member_expression.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index a3d1103598..14483f860d 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -292,7 +292,7 @@ func (interpreter *Interpreter) getReferenceValue(value Value, semaType sema.Typ } interpreter.maybeTrackReferencedResourceKindedValue(value) - return NewEphemeralReferenceValue(interpreter, false, value, semaType) + return NewEphemeralReferenceValue(interpreter, UnauthorizedAccess, value, semaType) } func (interpreter *Interpreter) checkMemberAccess( diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 7b9c5fbbba..c956c1e70f 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -122,7 +122,7 @@ func (checker *Checker) getReferenceType(typ Type) Type { } } - return NewReferenceType(checker.memoryGauge, typ, false) + return NewReferenceType(checker.memoryGauge, typ, UnauthorizedAccess) } func shouldReturnReference(parentType, memberType Type) bool { From 5c8ea343e51e568e3fd756f233c97436556c82af Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 19 Jun 2023 12:09:15 -0700 Subject: [PATCH 11/24] Re-use pre-computed results from checker --- runtime/interpreter/interpreter_expression.go | 51 ++++++------------- runtime/sema/check_expression.go | 4 ++ runtime/sema/check_member_expression.go | 13 +++++ runtime/sema/elaboration.go | 14 ++--- runtime/tests/interpreter/member_test.go | 47 +++++++++++++++++ 5 files changed, 87 insertions(+), 42 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 14483f860d..4b72edd43c 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -147,7 +147,7 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression value := target.GetKey(interpreter, locationRange, transferredIndexingValue) // If the indexing value is a reference, then return a reference for the resulting value. - return interpreter.maybeGetReference(indexExpression, target, value) + return interpreter.maybeGetReference(indexExpression, value) } }, set: func(value Value) { @@ -222,18 +222,8 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } // Return a reference, if the member is accessed via a reference. - // - // However, for attachments, `self` is always a reference. - // But we do not want to return a reference for `self.something`. - // Otherwise, things like `destroy self.something` would become invalid. - // Hence, special case `self`, and return a reference only if the member is not accessed via self. - - accessingSelf := false - if identifierExpression, ok := memberExpression.Expression.(*ast.IdentifierExpression); ok { - accessingSelf = identifierExpression.Identifier.Identifier == sema.SelfIdentifier - } - - if !accessingSelf && shouldReturnReference(target, resultValue) { + // This is pre-computed at the checker. + if memberInfo.ReturnReference { // Get a reference to the value resultValue = interpreter.getReferenceValue(resultValue, memberType) } @@ -248,31 +238,21 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a } } -func shouldReturnReference(parent, member Value) bool { - _, parentIsReference := parent.(ReferenceValue) - return parentIsReference && isContainerValue(member) -} - -func isContainerValue(value Value) bool { - switch value := value.(type) { - case *CompositeValue, - *SimpleCompositeValue, - *DictionaryValue, - *ArrayValue: - return true - case *SomeValue: - return isContainerValue(value.value) - default: - return false - } -} - // getReferenceValue Returns a reference to a given value. // Reference to an optional should return an optional reference. // This has to be done recursively for nested optionals. // e.g.1: Given type T, this method returns &T. // e.g.2: Given T?, this returns (&T)? func (interpreter *Interpreter) getReferenceValue(value Value, semaType sema.Type) Value { + switch value.(type) { + case NilValue: + // Reference to a nil, should return a nil. + return value + case ReferenceValue: + // If the value is already a reference then return the same reference. + return value + } + optionalType, ok := semaType.(*sema.OptionalType) if ok { semaType = optionalType.Type @@ -933,17 +913,16 @@ func (interpreter *Interpreter) VisitIndexExpression(expression *ast.IndexExpres value := typedResult.GetKey(interpreter, locationRange, indexingValue) // If the indexing value is a reference, then return a reference for the resulting value. - return interpreter.maybeGetReference(expression, typedResult, value) + return interpreter.maybeGetReference(expression, value) } } func (interpreter *Interpreter) maybeGetReference( expression *ast.IndexExpression, - parentValue ValueIndexableValue, memberValue Value, ) Value { - if shouldReturnReference(parentValue, memberValue) { - indexExpressionTypes := interpreter.Program.Elaboration.IndexExpressionTypes(expression) + indexExpressionTypes := interpreter.Program.Elaboration.IndexExpressionTypes(expression) + if indexExpressionTypes.ReturnReference { elementType := indexExpressionTypes.IndexedType.ElementType(false) // Get a reference to the value diff --git a/runtime/sema/check_expression.go b/runtime/sema/check_expression.go index 102891f8ce..abde2f3011 100644 --- a/runtime/sema/check_expression.go +++ b/runtime/sema/check_expression.go @@ -272,6 +272,10 @@ func (checker *Checker) VisitIndexExpression(expression *ast.IndexExpression) Ty // then the element type should also be a reference. if shouldReturnReference(parentType, elementType) { elementType = checker.getReferenceType(elementType) + + // Store the result in elaboration, so the interpreter can re-use this. + indexExprTypes.ReturnReference = true + checker.Elaboration.SetIndexExpressionTypes(expression, indexExprTypes) } return elementType diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index c956c1e70f..8a38cbfd21 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -21,6 +21,7 @@ package sema import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" + "github.com/onflow/cadence/runtime/errors" ) // NOTE: only called if the member expression is *not* an assignment @@ -105,6 +106,14 @@ func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) member.DeclarationKind == common.DeclarationKindField { // Get a reference to the type memberType = checker.getReferenceType(memberType) + + // Store the result in elaboration, so the interpreter can re-use this. + memberInfo, ok := checker.Elaboration.MemberExpressionMemberInfo(expression) + if !ok { + panic(errors.NewUnreachableError()) + } + memberInfo.ReturnReference = true + checker.Elaboration.SetMemberExpressionMemberInfo(expression, memberInfo) } return memberType @@ -148,6 +157,10 @@ func isContainerType(typ Type) bool { case *OptionalType: return isContainerType(typ.Type) default: + switch typ { + case AnyStructType, AnyResourceType: + return true + } return false } } diff --git a/runtime/sema/elaboration.go b/runtime/sema/elaboration.go index f0aba25c08..af2224ce45 100644 --- a/runtime/sema/elaboration.go +++ b/runtime/sema/elaboration.go @@ -26,10 +26,11 @@ import ( ) type MemberInfo struct { - AccessedType Type - ResultingType Type - Member *Member - IsOptional bool + AccessedType Type + ResultingType Type + Member *Member + IsOptional bool + ReturnReference bool } type CastTypes struct { @@ -88,8 +89,9 @@ type SwapStatementTypes struct { } type IndexExpressionTypes struct { - IndexedType ValueIndexableType - IndexingType Type + IndexedType ValueIndexableType + IndexingType Type + ReturnReference bool } type NumberConversionArgumentTypes struct { diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 81d542de19..71262fb7b2 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -726,6 +726,33 @@ func TestInterpretMemberAccess(t *testing.T) { require.NoError(t, err) }) + t.Run("composite reference, anystruct typed field, with reference value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + struct Test { + var x: AnyStruct + init() { + var s = "hello" + self.x = &s as &String + } + } + + fun test():&AnyStruct { + let test = Test() + let testRef = &test as &Test + return testRef.x + } + `) + + result, err := inter.Invoke("test") + require.NoError(t, err) + + require.IsType(t, &interpreter.EphemeralReferenceValue{}, result) + ref := result.(*interpreter.EphemeralReferenceValue) + + // Must only have one level of references. + require.IsType(t, &interpreter.StringValue{}, ref.Value) + }) + t.Run("array, element", func(t *testing.T) { inter := parseCheckAndInterpret(t, ` fun test() { @@ -791,6 +818,26 @@ func TestInterpretMemberAccess(t *testing.T) { require.NoError(t, err) }) + t.Run("array reference, anystruct typed element, with reference value", func(t *testing.T) { + inter := parseCheckAndInterpret(t, ` + fun test(): &AnyStruct { + var s = "hello" + let array: [AnyStruct] = [&s as &String] + let arrayRef = &array as &[AnyStruct] + return arrayRef[0] + } + `) + + result, err := inter.Invoke("test") + require.NoError(t, err) + + require.IsType(t, &interpreter.EphemeralReferenceValue{}, result) + ref := result.(*interpreter.EphemeralReferenceValue) + + // Must only have one level of references. + require.IsType(t, &interpreter.StringValue{}, ref.Value) + }) + t.Run("dictionary, value", func(t *testing.T) { inter := parseCheckAndInterpret(t, ` fun test() { From eca92dfc94ed739876e321a3b80464398c8a45db Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 19 Jun 2023 12:57:15 -0700 Subject: [PATCH 12/24] Improve tests --- runtime/attachments_test.go | 4 +- runtime/tests/checker/member_test.go | 32 ++++++++++ runtime/tests/interpreter/member_test.go | 80 ++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) diff --git a/runtime/attachments_test.go b/runtime/attachments_test.go index f77cadc816..246f032321 100644 --- a/runtime/attachments_test.go +++ b/runtime/attachments_test.go @@ -172,7 +172,9 @@ func TestAccountAttachmentExportFailure(t *testing.T) { let r <- Test.makeRWithA() var a = r[Test.A] - // just to trick the checker + // Life span of attachments (references) are validated statically. + // This indirection helps to trick the checker and causes to perform the validation at runtime, + // which is the intention of this test. a = returnSameRef(a) destroy r diff --git a/runtime/tests/checker/member_test.go b/runtime/tests/checker/member_test.go index bd5c866aa4..f24c23439b 100644 --- a/runtime/tests/checker/member_test.go +++ b/runtime/tests/checker/member_test.go @@ -424,6 +424,8 @@ func TestCheckMemberAccess(t *testing.T) { t.Parallel() t.Run("composite, field", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { var x: [Int] @@ -442,6 +444,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("composite, function", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { pub fun foo(): Int { @@ -459,6 +463,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("composite reference, field", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { var x: [Int] @@ -478,6 +484,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("composite reference, optional field", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { var x: [Int]? @@ -497,6 +505,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("composite reference, primitive field", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { var x: Int @@ -516,6 +526,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("composite reference, function", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` struct Test { pub fun foo(): Int { @@ -534,6 +546,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("array, element", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let array: [[Int]] = [[1, 2]] @@ -545,6 +559,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("array reference, element", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let array: [[Int]] = [[1, 2]] @@ -557,6 +573,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("array reference, optional typed element", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let array: [[Int]?] = [[1, 2]] @@ -569,6 +587,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("array reference, primitive typed element", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let array: [Int] = [1, 2] @@ -581,6 +601,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("dictionary, value", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let dict: {String: {String: Int}} = {"one": {"two": 2}} @@ -592,6 +614,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("dictionary reference, value", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let dict: {String: {String: Int} } = {"one": {"two": 2}} @@ -604,6 +628,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("dictionary reference, optional typed value", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let dict: {String: {String: Int}?} = {"one": {"two": 2}} @@ -616,6 +642,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("dictionary reference, optional typed value, mismatch types", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let dict: {String: {String: Int}?} = {"one": {"two": 2}} @@ -632,6 +660,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("dictionary reference, primitive typed value", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` fun test() { let dict: {String: Int} = {"one": 1} @@ -644,6 +674,8 @@ func TestCheckMemberAccess(t *testing.T) { }) t.Run("resource reference, attachment", func(t *testing.T) { + t.Parallel() + _, err := ParseAndCheck(t, ` resource R {} diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 71262fb7b2..b008bc5c33 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -563,6 +563,8 @@ func TestInterpretMemberAccess(t *testing.T) { t.Parallel() t.Run("composite, field", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { var x: [Int] @@ -582,6 +584,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite, function", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { pub fun foo(): Int { @@ -600,6 +604,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite reference, field", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { var x: [Int] @@ -620,6 +626,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite reference, optional field", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { var x: [Int]? @@ -640,6 +648,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite reference, primitive field", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { var x: Int @@ -660,6 +670,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite reference, function", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { pub fun foo(): Int { @@ -679,6 +691,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("resource reference, nested", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` resource Foo { var bar: @Bar @@ -727,6 +741,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("composite reference, anystruct typed field, with reference value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` struct Test { var x: AnyStruct @@ -754,6 +770,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array, element", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let array: [[Int]] = [[1, 2]] @@ -766,6 +784,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array reference, element", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let array: [[Int]] = [[1, 2]] @@ -779,6 +799,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array reference, element, in assignment", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let array: [[Int]] = [[1, 2]] @@ -793,6 +815,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array reference, optional typed element", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let array: [[Int]?] = [[1, 2]] @@ -806,6 +830,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array reference, primitive typed element", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let array: [Int] = [1, 2] @@ -819,6 +845,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("array reference, anystruct typed element, with reference value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test(): &AnyStruct { var s = "hello" @@ -839,6 +867,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("dictionary, value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let dict: {String: {String: Int}} = {"one": {"two": 2}} @@ -851,6 +881,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("dictionary reference, value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let dict: {String: {String: Int} } = {"one": {"two": 2}} @@ -864,6 +896,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("dictionary reference, value, in assignment", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let dict: {String: {String: Int} } = {"one": {"two": 2}} @@ -878,6 +912,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("dictionary reference, optional typed value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let dict: {String: {String: Int}?} = {"one": {"two": 2}} @@ -891,6 +927,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("dictionary reference, primitive typed value", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` fun test() { let dict: {String: Int} = {"one": 1} @@ -904,6 +942,8 @@ func TestInterpretMemberAccess(t *testing.T) { }) t.Run("resource reference, attachment", func(t *testing.T) { + t.Parallel() + inter := parseCheckAndInterpret(t, ` resource R {} @@ -921,4 +961,44 @@ func TestInterpretMemberAccess(t *testing.T) { _, err := inter.Invoke("test") require.NoError(t, err) }) + + t.Run("attachment nested member", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + resource R {} + + attachment A for R { + var foo: Foo + init() { + self.foo = Foo() + } + + access(all) fun getNestedMember(): [Int] { + return self.foo.array + } + } + + struct Foo { + var array: [Int] + init() { + self.array = [] + } + } + + fun test() { + let r <- attach A() to <- create R() + let rRef = &r as &R + + var a: &A? = rRef[A] + + var array = a!.getNestedMember() + + destroy r + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) } From 498c63a860a8d24c8f5765689416d33705499573 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 19 Jun 2023 13:16:54 -0700 Subject: [PATCH 13/24] Add test for index expression resource loss --- runtime/tests/checker/resources_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/runtime/tests/checker/resources_test.go b/runtime/tests/checker/resources_test.go index a464a8a773..5b1432893e 100644 --- a/runtime/tests/checker/resources_test.go +++ b/runtime/tests/checker/resources_test.go @@ -9447,3 +9447,21 @@ func TestCheckConditionalResourceCreationAndReturn(t *testing.T) { require.NoError(t, err) } + +func TestCheckIndexExpressionResourceLoss(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource R {} + + fun test() { + let rs <- [<-create R()] + rs[0] + destroy rs + } + `) + + errs := RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.ResourceLossError{}, errs[0]) +} From 515771146be89d260d5bb0a3f129dee199907973 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 19 Jun 2023 16:31:47 -0700 Subject: [PATCH 14/24] Simplify reference creation --- runtime/sema/check_member_expression.go | 58 ++++++++++++------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 8a38cbfd21..26d9ee0f63 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -21,7 +21,6 @@ package sema import ( "github.com/onflow/cadence/runtime/ast" "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/errors" ) // NOTE: only called if the member expression is *not* an assignment @@ -90,32 +89,6 @@ func (checker *Checker) VisitMemberExpression(expression *ast.MemberExpression) } } - // If the member, - // 1) is accessed via a reference, and - // 2) is container-typed, - // then the member type should also be a reference. - - // Note: For attachments, `self` is always a reference. - // But we do not want to return a reference for `self.something`. - // Otherwise, things like `destroy self.something` would become invalid. - // Hence, special case `self`, and return a reference only if the member is not accessed via self. - // i.e: `accessedSelfMember == nil` - - if accessedSelfMember == nil && - shouldReturnReference(accessedType, memberType) && - member.DeclarationKind == common.DeclarationKindField { - // Get a reference to the type - memberType = checker.getReferenceType(memberType) - - // Store the result in elaboration, so the interpreter can re-use this. - memberInfo, ok := checker.Elaboration.MemberExpressionMemberInfo(expression) - if !ok { - panic(errors.NewUnreachableError()) - } - memberInfo.ReturnReference = true - checker.Elaboration.SetMemberExpressionMemberInfo(expression, memberInfo) - } - return memberType } @@ -171,14 +144,17 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT return memberInfo.AccessedType, memberInfo.ResultingType, memberInfo.Member, memberInfo.IsOptional } + returnReference := false + defer func() { checker.Elaboration.SetMemberExpressionMemberInfo( expression, MemberInfo{ - AccessedType: accessedType, - ResultingType: resultingType, - Member: member, - IsOptional: isOptional, + AccessedType: accessedType, + ResultingType: resultingType, + Member: member, + IsOptional: isOptional, + ReturnReference: returnReference, }, ) }() @@ -398,6 +374,26 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT ) } } + + // If the member, + // 1) is accessed via a reference, and + // 2) is container-typed, + // then the member type should also be a reference. + + // Note: For attachments, `self` is always a reference. + // But we do not want to return a reference for `self.something`. + // Otherwise, things like `destroy self.something` would become invalid. + // Hence, special case `self`, and return a reference only if the member is not accessed via self. + // i.e: `accessedSelfMember == nil` + + if accessedSelfMember == nil && + shouldReturnReference(accessedType, resultingType) && + member.DeclarationKind == common.DeclarationKindField { + // Get a reference to the type + resultingType = checker.getReferenceType(resultingType) + returnReference = true + } + return accessedType, resultingType, member, isOptional } From dadf1c39244f5d0cbb0e6a671f1a00e9d62b430e Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Mon, 19 Jun 2023 16:48:52 -0700 Subject: [PATCH 15/24] Simplify reference creation for index expressions --- runtime/sema/check_expression.go | 39 +++++++++++++------------------- 1 file changed, 16 insertions(+), 23 deletions(-) diff --git a/runtime/sema/check_expression.go b/runtime/sema/check_expression.go index abde2f3011..c5d279df1d 100644 --- a/runtime/sema/check_expression.go +++ b/runtime/sema/check_expression.go @@ -258,27 +258,7 @@ func (checker *Checker) VisitStringExpression(expression *ast.StringExpression) } func (checker *Checker) VisitIndexExpression(expression *ast.IndexExpression) Type { - elementType := checker.visitIndexExpression(expression, false) - if elementType == InvalidType { - return elementType - } - - indexExprTypes := checker.Elaboration.IndexExpressionTypes(expression) - parentType := indexExprTypes.IndexedType - - // If the element, - // 1) is accessed via a reference, and - // 2) is container-typed, - // then the element type should also be a reference. - if shouldReturnReference(parentType, elementType) { - elementType = checker.getReferenceType(elementType) - - // Store the result in elaboration, so the interpreter can re-use this. - indexExprTypes.ReturnReference = true - checker.Elaboration.SetIndexExpressionTypes(expression, indexExprTypes) - } - - return elementType + return checker.visitIndexExpression(expression, false) } // visitIndexExpression checks if the indexed expression is indexable, @@ -336,11 +316,24 @@ func (checker *Checker) visitIndexExpression( checker.checkUnusedExpressionResourceLoss(elementType, targetExpression) + // If the element, + // 1) is accessed via a reference, and + // 2) is container-typed, + // then the element type should also be a reference. + returnReference := false + if !isAssignment && shouldReturnReference(valueIndexedType, elementType) { + elementType = checker.getReferenceType(elementType) + + // Store the result in elaboration, so the interpreter can re-use this. + returnReference = true + } + checker.Elaboration.SetIndexExpressionTypes( indexExpression, IndexExpressionTypes{ - IndexedType: valueIndexedType, - IndexingType: indexingType, + IndexedType: valueIndexedType, + IndexingType: indexingType, + ReturnReference: returnReference, }, ) From b35e6a87b2b1d3e577b4bed29e2504498c594e29 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 21 Jun 2023 16:21:40 -0700 Subject: [PATCH 16/24] Replace 'pub' with 'access(all)' --- runtime/attachments_test.go | 2 +- runtime/tests/checker/member_test.go | 4 ++-- runtime/tests/checker/reference_test.go | 14 +++++++------- runtime/tests/interpreter/attachments_test.go | 8 ++++---- runtime/tests/interpreter/member_test.go | 4 ++-- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/runtime/attachments_test.go b/runtime/attachments_test.go index 246f032321..505211cbc8 100644 --- a/runtime/attachments_test.go +++ b/runtime/attachments_test.go @@ -181,7 +181,7 @@ func TestAccountAttachmentExportFailure(t *testing.T) { return a } - pub fun returnSameRef(_ ref: &Test.A?): &Test.A? { + access(all) fun returnSameRef(_ ref: &Test.A?): &Test.A? { return ref } `) diff --git a/runtime/tests/checker/member_test.go b/runtime/tests/checker/member_test.go index f24c23439b..b14339c3bc 100644 --- a/runtime/tests/checker/member_test.go +++ b/runtime/tests/checker/member_test.go @@ -448,7 +448,7 @@ func TestCheckMemberAccess(t *testing.T) { _, err := ParseAndCheck(t, ` struct Test { - pub fun foo(): Int { + access(all) fun foo(): Int { return 1 } } @@ -530,7 +530,7 @@ func TestCheckMemberAccess(t *testing.T) { _, err := ParseAndCheck(t, ` struct Test { - pub fun foo(): Int { + access(all) fun foo(): Int { return 1 } } diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go index c0d3a6e3ec..c4cd43a97e 100644 --- a/runtime/tests/checker/reference_test.go +++ b/runtime/tests/checker/reference_test.go @@ -1897,7 +1897,7 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { _, err := ParseAndCheck(t, ` - pub fun test() { + access(all) fun test() { var r: @R1 <- create R1() let ref1 = &r as &R1 let ref2 = ref1.r2 @@ -1906,8 +1906,8 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { ref3.a } - pub resource R1 { - pub let r2: @R2 + access(all) resource R1 { + access(all) let r2: @R2 init() { self.r2 <- create R2() } @@ -1916,8 +1916,8 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { } } - pub resource R2 { - pub let r3: @R3 + access(all) resource R2 { + access(all) let r3: @R3 init() { self.r3 <- create R3() } @@ -1926,8 +1926,8 @@ func TestCheckInvalidatedReferenceUse(t *testing.T) { } } - pub resource R3 { - pub let a: Int + access(all) resource R3 { + access(all) let a: Int init() { self.a = 5 } diff --git a/runtime/tests/interpreter/attachments_test.go b/runtime/tests/interpreter/attachments_test.go index 840baf4d38..6319de0aa6 100644 --- a/runtime/tests/interpreter/attachments_test.go +++ b/runtime/tests/interpreter/attachments_test.go @@ -1592,7 +1592,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { return a.id } - pub fun returnSameRef(_ ref: &A): &A { + access(all) fun returnSameRef(_ ref: &A): &A { return ref }`, sema.Config{ @@ -1623,7 +1623,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { let i = a.foo() } - pub fun returnSameRef(_ ref: &A): &A { + access(all) fun returnSameRef(_ ref: &A): &A { return ref }`, sema.Config{ @@ -1673,7 +1673,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { return a.id } - pub fun returnSameRef(_ ref: &A): &A { + access(all) fun returnSameRef(_ ref: &A): &A { return ref }`, sema.Config{ @@ -1754,7 +1754,7 @@ func TestInterpretAttachmentResourceReferenceInvalidation(t *testing.T) { let i = a.foo() } - pub fun returnSameRef(_ ref: &A): &A { + access(all) fun returnSameRef(_ ref: &A): &A { return ref }`, sema.Config{ diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index b008bc5c33..ed0a9f401b 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -588,7 +588,7 @@ func TestInterpretMemberAccess(t *testing.T) { inter := parseCheckAndInterpret(t, ` struct Test { - pub fun foo(): Int { + access(all) fun foo(): Int { return 1 } } @@ -674,7 +674,7 @@ func TestInterpretMemberAccess(t *testing.T) { inter := parseCheckAndInterpret(t, ` struct Test { - pub fun foo(): Int { + access(all) fun foo(): Int { return 1 } } From ade5bba34d14ee4ab81289e8a7db3df1a377c4e3 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 22 Jun 2023 14:09:00 -0700 Subject: [PATCH 17/24] Update test --- runtime/tests/checker/reference_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/tests/checker/reference_test.go b/runtime/tests/checker/reference_test.go index 4201ea375e..f3ca55e029 100644 --- a/runtime/tests/checker/reference_test.go +++ b/runtime/tests/checker/reference_test.go @@ -2825,7 +2825,7 @@ func TestCheckReferenceUseAfterCopy(t *testing.T) { resource R {} attachment A for R { - pub(set) var id: UInt8 + access(all) var id: UInt8 init() { self.id = 1 } From fa638aecac693a1727651bb0ddd96fff700e0996 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 22 Jun 2023 15:23:12 -0700 Subject: [PATCH 18/24] Fix swap statement --- runtime/resource_duplicate_test.go | 17 ++++++--- runtime/sema/check_swap.go | 46 ++++++++---------------- runtime/sema/type.go | 2 +- runtime/tests/checker/member_test.go | 21 +++++++++++ runtime/tests/checker/swap_test.go | 10 +++--- runtime/tests/interpreter/member_test.go | 23 ++++++++++++ 6 files changed, 78 insertions(+), 41 deletions(-) diff --git a/runtime/resource_duplicate_test.go b/runtime/resource_duplicate_test.go index 743cf198bf..0c03021eab 100644 --- a/runtime/resource_duplicate_test.go +++ b/runtime/resource_duplicate_test.go @@ -168,7 +168,12 @@ func TestRuntimeResourceDuplicationUsingDestructorIteration(t *testing.T) { }, ) - require.ErrorAs(t, err, &interpreter.ContainerMutatedDuringIterationError{}) + var checkerErr *sema.CheckerError + require.ErrorAs(t, err, &checkerErr) + + errs := checker.RequireCheckerErrors(t, checkerErr, 2) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) }) t.Run("simplified", func(t *testing.T) { @@ -268,10 +273,11 @@ func TestRuntimeResourceDuplicationUsingDestructorIteration(t *testing.T) { var checkerErr *sema.CheckerError require.ErrorAs(t, err, &checkerErr) - errs := checker.RequireCheckerErrors(t, checkerErr, 1) - - assert.IsType(t, &sema.InvalidatedResourceReferenceError{}, errs[0]) + errs := checker.RequireCheckerErrors(t, checkerErr, 3) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) + assert.IsType(t, &sema.InvalidatedResourceReferenceError{}, errs[2]) }) t.Run("forEachKey", func(t *testing.T) { @@ -355,7 +361,8 @@ func TestRuntimeResourceDuplicationUsingDestructorIteration(t *testing.T) { }, ) - require.ErrorAs(t, err, &interpreter.ContainerMutatedDuringIterationError{}) + errs := checker.RequireCheckerErrors(t, err, 1) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) }) t.Run("array", func(t *testing.T) { diff --git a/runtime/sema/check_swap.go b/runtime/sema/check_swap.go index 2724a7e8f1..da30e6df50 100644 --- a/runtime/sema/check_swap.go +++ b/runtime/sema/check_swap.go @@ -25,38 +25,28 @@ import ( func (checker *Checker) VisitSwapStatement(swap *ast.SwapStatement) (_ struct{}) { - leftType := checker.VisitExpression(swap.Left, nil) - rightType := checker.VisitExpression(swap.Right, nil) + // First visit the two expressions as if they were the target of the assignment. + leftTargetType := checker.checkSwapStatementExpression(swap.Left, common.OperandSideLeft) + rightTargetType := checker.checkSwapStatementExpression(swap.Right, common.OperandSideRight) + + // Then re-visit the same expressions, this time treat them as the value-expr of the assignment. + // The 'expected type' of the two expression would be the types obtained from the previous visit, swapped. + leftValueType := checker.VisitExpression(swap.Left, rightTargetType) + rightValueType := checker.VisitExpression(swap.Right, leftTargetType) checker.Elaboration.SetSwapStatementTypes( swap, SwapStatementTypes{ - LeftType: leftType, - RightType: rightType, + LeftType: leftValueType, + RightType: rightValueType, }, ) - lhsValid := checker.checkSwapStatementExpression(swap.Left, leftType, common.OperandSideLeft) - rhsValid := checker.checkSwapStatementExpression(swap.Right, rightType, common.OperandSideRight) - - // The types of both sides must be subtypes of each other, - // so that assignment can be performed in both directions. - // i.e: The two types have to be equal. - if lhsValid && rhsValid && !leftType.Equal(rightType) { - checker.report( - &TypeMismatchError{ - ExpectedType: leftType, - ActualType: rightType, - Range: ast.NewRangeFromPositioned(checker.memoryGauge, swap.Right), - }, - ) - } - - if leftType.IsResourceType() { + if leftValueType.IsResourceType() { checker.elaborateNestedResourceMoveExpression(swap.Left) } - if rightType.IsResourceType() { + if rightValueType.IsResourceType() { checker.elaborateNestedResourceMoveExpression(swap.Right) } @@ -65,9 +55,8 @@ func (checker *Checker) VisitSwapStatement(swap *ast.SwapStatement) (_ struct{}) func (checker *Checker) checkSwapStatementExpression( expression ast.Expression, - exprType Type, opSide common.OperandSide, -) bool { +) Type { // Expression in either side of the swap statement must be a target expression. // (e.g. identifier expression, indexing expression, or member access expression) @@ -78,13 +67,8 @@ func (checker *Checker) checkSwapStatementExpression( Range: ast.NewRangeFromPositioned(checker.memoryGauge, expression), }, ) - return false - } - - if exprType.IsInvalidType() { - return false + return InvalidType } - checker.visitAssignmentValueType(expression) - return true + return checker.visitAssignmentValueType(expression) } diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 5f4cd6bf16..73b53df7ba 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -5437,7 +5437,7 @@ func (*DictionaryType) isValueIndexableType() bool { return true } -func (t *DictionaryType) ElementType(_ bool) Type { +func (t *DictionaryType) ElementType(isAssignment bool) Type { return &OptionalType{Type: t.ValueType} } diff --git a/runtime/tests/checker/member_test.go b/runtime/tests/checker/member_test.go index b14339c3bc..289b6c7026 100644 --- a/runtime/tests/checker/member_test.go +++ b/runtime/tests/checker/member_test.go @@ -692,4 +692,25 @@ func TestCheckMemberAccess(t *testing.T) { require.NoError(t, err) }) + + t.Run("anyresource swap on reference", func(t *testing.T) { + t.Parallel() + + _, err := ParseAndCheck(t, ` + resource Foo {} + + fun test() { + let dict: @{String: AnyResource} <- {"foo": <- create Foo(), "bar": <- create Foo()} + let dictRef = &dict as &{String: AnyResource} + + dictRef["foo"] <-> dictRef["bar"] + + destroy dict + } + `) + + errs := RequireCheckerErrors(t, err, 2) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + }) } diff --git a/runtime/tests/checker/swap_test.go b/runtime/tests/checker/swap_test.go index adf4a2579b..3e35af8884 100644 --- a/runtime/tests/checker/swap_test.go +++ b/runtime/tests/checker/swap_test.go @@ -39,9 +39,9 @@ func TestCheckInvalidUnknownDeclarationSwap(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 1) - + errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.NotDeclaredError{}, errs[0]) + assert.IsType(t, &sema.NotDeclaredError{}, errs[1]) } func TestCheckInvalidLeftConstantSwap(t *testing.T) { @@ -105,9 +105,10 @@ func TestCheckInvalidTypesSwap(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) } func TestCheckInvalidTypesSwap2(t *testing.T) { @@ -122,9 +123,10 @@ func TestCheckInvalidTypesSwap2(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) } func TestCheckInvalidSwapTargetExpressionLeft(t *testing.T) { diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index ed0a9f401b..68d6f491e6 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -1001,4 +1001,27 @@ func TestInterpretMemberAccess(t *testing.T) { _, err := inter.Invoke("test") require.NoError(t, err) }) + + t.Run("anystruct swap on reference", func(t *testing.T) { + t.Parallel() + + inter := parseCheckAndInterpret(t, ` + struct Foo { + var array: [Int] + init() { + self.array = [] + } + } + + fun test() { + let dict: {String: AnyStruct} = {"foo": Foo(), "bar": Foo()} + let dictRef = &dict as &{String: AnyStruct} + + dictRef["foo"] <-> dictRef["bar"] + } + `) + + _, err := inter.Invoke("test") + require.NoError(t, err) + }) } From 7cfcc05ee6a5bacf9e8a4ecc7696929034e3771e Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 23 Jun 2023 14:49:59 -0700 Subject: [PATCH 19/24] Move is-container check to the type itself --- runtime/interpreter/interpreter_expression.go | 4 +- .../sema/account_capability_controller.cdc | 2 +- .../sema/account_capability_controller.gen.go | 21 ++--- runtime/sema/anyresource_type.go | 5 +- runtime/sema/anystruct_type.go | 3 +- runtime/sema/authaccount.cdc | 2 +- runtime/sema/block.cdc | 2 +- runtime/sema/block.gen.go | 21 ++--- runtime/sema/check_member_expression.go | 19 +---- runtime/sema/deployedcontract.cdc | 2 +- runtime/sema/deployedcontract.gen.go | 21 ++--- runtime/sema/gen/main.go | 11 +++ runtime/sema/simple_type.go | 5 ++ .../sema/storage_capability_controller.cdc | 2 +- .../sema/storage_capability_controller.gen.go | 21 ++--- runtime/sema/type.go | 78 ++++++++++++++++++- runtime/tests/checker/member_test.go | 54 ++++++++++++- runtime/tests/interpreter/member_test.go | 55 +++++++++++++ 18 files changed, 255 insertions(+), 73 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 1b86c3f65b..96ebed3636 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -245,10 +245,8 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a // e.g.2: Given T?, this returns (&T)? func (interpreter *Interpreter) getReferenceValue(value Value, semaType sema.Type) Value { switch value.(type) { - case NilValue: + case NilValue, ReferenceValue: // Reference to a nil, should return a nil. - return value - case ReferenceValue: // If the value is already a reference then return the same reference. return value } diff --git a/runtime/sema/account_capability_controller.cdc b/runtime/sema/account_capability_controller.cdc index e1813cfe0d..2d3e9f9b64 100644 --- a/runtime/sema/account_capability_controller.cdc +++ b/runtime/sema/account_capability_controller.cdc @@ -1,4 +1,4 @@ -access(all) struct AccountCapabilityController { +access(all) struct AccountCapabilityController: MemberAccessible { /// An arbitrary "tag" for the controller. /// For example, it could be used to describe the purpose of the capability. diff --git a/runtime/sema/account_capability_controller.gen.go b/runtime/sema/account_capability_controller.gen.go index 2d2d0841a9..a71550bd63 100644 --- a/runtime/sema/account_capability_controller.gen.go +++ b/runtime/sema/account_capability_controller.gen.go @@ -91,16 +91,17 @@ Borrowing from the controlled capability or its copies will return nil. const AccountCapabilityControllerTypeName = "AccountCapabilityController" var AccountCapabilityControllerType = &SimpleType{ - Name: AccountCapabilityControllerTypeName, - QualifiedName: AccountCapabilityControllerTypeName, - TypeID: AccountCapabilityControllerTypeName, - tag: AccountCapabilityControllerTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: AccountCapabilityControllerTypeName, + QualifiedName: AccountCapabilityControllerTypeName, + TypeID: AccountCapabilityControllerTypeName, + tag: AccountCapabilityControllerTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: true, } func init() { diff --git a/runtime/sema/anyresource_type.go b/runtime/sema/anyresource_type.go index d1de774aa8..19e41d3133 100644 --- a/runtime/sema/anyresource_type.go +++ b/runtime/sema/anyresource_type.go @@ -30,6 +30,7 @@ var AnyResourceType = &SimpleType{ Equatable: false, Comparable: false, // The actual returnability of a value is checked at run-time - Exportable: true, - Importable: false, + Exportable: true, + Importable: false, + MemberAccessible: true, } diff --git a/runtime/sema/anystruct_type.go b/runtime/sema/anystruct_type.go index 14548e0eeb..339d81df20 100644 --- a/runtime/sema/anystruct_type.go +++ b/runtime/sema/anystruct_type.go @@ -31,7 +31,8 @@ var AnyStructType = &SimpleType{ Comparable: false, Exportable: true, // The actual importability is checked at runtime - Importable: true, + Importable: true, + MemberAccessible: true, } var AnyStructTypeAnnotation = NewTypeAnnotation(AnyStructType) diff --git a/runtime/sema/authaccount.cdc b/runtime/sema/authaccount.cdc index e987a4094b..391263a5f4 100644 --- a/runtime/sema/authaccount.cdc +++ b/runtime/sema/authaccount.cdc @@ -1,5 +1,5 @@ -access(all) struct AuthAccount { +access(all) struct AuthAccount: MemberAccessible { /// The address of the account. access(all) let address: Address diff --git a/runtime/sema/block.cdc b/runtime/sema/block.cdc index df8562d954..f2b25075b7 100644 --- a/runtime/sema/block.cdc +++ b/runtime/sema/block.cdc @@ -1,5 +1,5 @@ -access(all) struct Block { +access(all) struct Block: MemberAccessible { /// The height of the block. /// diff --git a/runtime/sema/block.gen.go b/runtime/sema/block.gen.go index 0e1b5d11cb..f74a58e29c 100644 --- a/runtime/sema/block.gen.go +++ b/runtime/sema/block.gen.go @@ -66,16 +66,17 @@ It is essentially the hash of the block const BlockTypeName = "Block" var BlockType = &SimpleType{ - Name: BlockTypeName, - QualifiedName: BlockTypeName, - TypeID: BlockTypeName, - tag: BlockTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: BlockTypeName, + QualifiedName: BlockTypeName, + TypeID: BlockTypeName, + tag: BlockTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: true, } func init() { diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 26d9ee0f63..da1f637776 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -112,7 +112,7 @@ func shouldReturnReference(parentType, memberType Type) bool { return false } - return isContainerType(memberType) + return memberType.IsMemberAccessible() } func isReferenceType(typ Type) bool { @@ -121,23 +121,6 @@ func isReferenceType(typ Type) bool { return isReference } -func isContainerType(typ Type) bool { - switch typ := typ.(type) { - case *CompositeType, - *DictionaryType, - ArrayType: - return true - case *OptionalType: - return isContainerType(typ.Type) - default: - switch typ { - case AnyStructType, AnyResourceType: - return true - } - return false - } -} - func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedType Type, resultingType Type, member *Member, isOptional bool) { memberInfo, ok := checker.Elaboration.MemberExpressionMemberInfo(expression) if ok { diff --git a/runtime/sema/deployedcontract.cdc b/runtime/sema/deployedcontract.cdc index 61f2195aa8..f3e824243a 100644 --- a/runtime/sema/deployedcontract.cdc +++ b/runtime/sema/deployedcontract.cdc @@ -1,5 +1,5 @@ -access(all) struct DeployedContract { +access(all) struct DeployedContract: MemberAccessible { /// The address of the account where the contract is deployed at. access(all) let address: Address diff --git a/runtime/sema/deployedcontract.gen.go b/runtime/sema/deployedcontract.gen.go index 87b3883b84..f473d72c81 100644 --- a/runtime/sema/deployedcontract.gen.go +++ b/runtime/sema/deployedcontract.gen.go @@ -74,16 +74,17 @@ then ` + "`.publicTypes()`" + ` will return an array equivalent to the expressio const DeployedContractTypeName = "DeployedContract" var DeployedContractType = &SimpleType{ - Name: DeployedContractTypeName, - QualifiedName: DeployedContractTypeName, - TypeID: DeployedContractTypeName, - tag: DeployedContractTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: DeployedContractTypeName, + QualifiedName: DeployedContractTypeName, + TypeID: DeployedContractTypeName, + tag: DeployedContractTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: true, } func init() { diff --git a/runtime/sema/gen/main.go b/runtime/sema/gen/main.go index eaf9d25473..5c6d2fdf11 100644 --- a/runtime/sema/gen/main.go +++ b/runtime/sema/gen/main.go @@ -151,6 +151,7 @@ type typeDecl struct { exportable bool comparable bool importable bool + memberAccessible bool memberDeclarations []ast.Declaration nestedTypes []*typeDecl } @@ -423,6 +424,15 @@ func (g *generator) VisitCompositeDeclaration(decl *ast.CompositeDeclaration) (_ case "Importable": typeDecl.importable = true + + case "MemberAccessible": + if !canGenerateSimpleType { + panic(fmt.Errorf( + "composite types cannot be explicitly marked as member accessible: %s", + g.currentTypeID(), + )) + } + typeDecl.memberAccessible = true } } @@ -1158,6 +1168,7 @@ func simpleTypeLiteral(ty *typeDecl) dst.Expr { goKeyValue("Comparable", goBoolLit(ty.comparable)), goKeyValue("Exportable", goBoolLit(ty.exportable)), goKeyValue("Importable", goBoolLit(ty.importable)), + goKeyValue("MemberAccessible", goBoolLit(ty.memberAccessible)), } return &dst.UnaryExpr{ diff --git a/runtime/sema/simple_type.go b/runtime/sema/simple_type.go index 049e0b29ed..cf411991e8 100644 --- a/runtime/sema/simple_type.go +++ b/runtime/sema/simple_type.go @@ -50,6 +50,7 @@ type SimpleType struct { Comparable bool Storable bool IsResource bool + MemberAccessible bool } var _ Type = &SimpleType{} @@ -106,6 +107,10 @@ func (t *SimpleType) IsImportable(_ map[*Member]bool) bool { return t.Importable } +func (t *SimpleType) IsMemberAccessible() bool { + return t.MemberAccessible +} + func (*SimpleType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } diff --git a/runtime/sema/storage_capability_controller.cdc b/runtime/sema/storage_capability_controller.cdc index 7d70961630..0f9ba83dba 100644 --- a/runtime/sema/storage_capability_controller.cdc +++ b/runtime/sema/storage_capability_controller.cdc @@ -1,4 +1,4 @@ -access(all) struct StorageCapabilityController { +access(all) struct StorageCapabilityController: MemberAccessible { /// An arbitrary "tag" for the controller. /// For example, it could be used to describe the purpose of the capability. diff --git a/runtime/sema/storage_capability_controller.gen.go b/runtime/sema/storage_capability_controller.gen.go index ac353f248e..ec12b21b75 100644 --- a/runtime/sema/storage_capability_controller.gen.go +++ b/runtime/sema/storage_capability_controller.gen.go @@ -123,16 +123,17 @@ The path may be different or the same as the current path. const StorageCapabilityControllerTypeName = "StorageCapabilityController" var StorageCapabilityControllerType = &SimpleType{ - Name: StorageCapabilityControllerTypeName, - QualifiedName: StorageCapabilityControllerTypeName, - TypeID: StorageCapabilityControllerTypeName, - tag: StorageCapabilityControllerTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: StorageCapabilityControllerTypeName, + QualifiedName: StorageCapabilityControllerTypeName, + TypeID: StorageCapabilityControllerTypeName, + tag: StorageCapabilityControllerTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: true, } func init() { diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 73b53df7ba..7613abb3af 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -135,6 +135,10 @@ type Type interface { // IsComparable returns true if values of the type can be compared IsComparable() bool + // IsMemberAccessible returns true if value of the type can have members values. + // Examples are composite types, restricted types, arrays, dictionaries, etc. + IsMemberAccessible() bool + TypeAnnotationState() TypeAnnotationState RewriteWithRestrictedTypes() (result Type, rewritten bool) @@ -638,6 +642,10 @@ func (*OptionalType) IsComparable() bool { return false } +func (t *OptionalType) IsMemberAccessible() bool { + return t.Type.IsMemberAccessible() +} + func (t *OptionalType) TypeAnnotationState() TypeAnnotationState { return t.Type.TypeAnnotationState() } @@ -843,6 +851,10 @@ func (*GenericType) IsComparable() bool { return false } +func (t *GenericType) IsMemberAccessible() bool { + return false +} + func (*GenericType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -1142,6 +1154,10 @@ func (t *NumericType) IsComparable() bool { return !t.IsSuperType() } +func (t *NumericType) IsMemberAccessible() bool { + return false +} + func (*NumericType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -1342,6 +1358,10 @@ func (t *FixedPointNumericType) IsComparable() bool { return !t.IsSuperType() } +func (t *FixedPointNumericType) IsMemberAccessible() bool { + return false +} + func (*FixedPointNumericType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -2402,6 +2422,10 @@ func (t *VariableSizedType) IsComparable() bool { return t.Type.IsComparable() } +func (t *VariableSizedType) IsMemberAccessible() bool { + return true +} + func (t *VariableSizedType) TypeAnnotationState() TypeAnnotationState { return t.Type.TypeAnnotationState() } @@ -2552,6 +2576,10 @@ func (t *ConstantSizedType) IsComparable() bool { return t.Type.IsComparable() } +func (t *ConstantSizedType) IsMemberAccessible() bool { + return true +} + func (t *ConstantSizedType) TypeAnnotationState() TypeAnnotationState { return t.Type.TypeAnnotationState() } @@ -3073,6 +3101,10 @@ func (*FunctionType) IsComparable() bool { return false } +func (*FunctionType) IsMemberAccessible() bool { + return false +} + func (t *FunctionType) TypeAnnotationState() TypeAnnotationState { for _, typeParameter := range t.TypeParameters { @@ -4244,8 +4276,12 @@ func (*CompositeType) IsComparable() bool { return false } -func (c *CompositeType) TypeAnnotationState() TypeAnnotationState { - if c.Kind == common.CompositeKindAttachment { +func (*CompositeType) IsMemberAccessible() bool { + return true +} + +func (t *CompositeType) TypeAnnotationState() TypeAnnotationState { + if t.Kind == common.CompositeKindAttachment { return TypeAnnotationStateDirectAttachmentTypeAnnotation } return TypeAnnotationStateValid @@ -4928,6 +4964,10 @@ func (*InterfaceType) IsComparable() bool { return false } +func (*InterfaceType) IsMemberAccessible() bool { + return true +} + func (*InterfaceType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -5145,6 +5185,10 @@ func (*DictionaryType) IsComparable() bool { return false } +func (*DictionaryType) IsMemberAccessible() bool { + return true +} + func (t *DictionaryType) TypeAnnotationState() TypeAnnotationState { keyTypeAnnotationState := t.KeyType.TypeAnnotationState() if keyTypeAnnotationState != TypeAnnotationStateValid { @@ -5437,7 +5481,7 @@ func (*DictionaryType) isValueIndexableType() bool { return true } -func (t *DictionaryType) ElementType(isAssignment bool) Type { +func (t *DictionaryType) ElementType(_ bool) Type { return &OptionalType{Type: t.ValueType} } @@ -5610,6 +5654,10 @@ func (*ReferenceType) IsComparable() bool { return false } +func (*ReferenceType) IsMemberAccessible() bool { + return false +} + func (r *ReferenceType) TypeAnnotationState() TypeAnnotationState { if r.Type.TypeAnnotationState() == TypeAnnotationStateDirectEntitlementTypeAnnotation { return TypeAnnotationStateDirectEntitlementTypeAnnotation @@ -5809,6 +5857,10 @@ func (*AddressType) IsComparable() bool { return false } +func (*AddressType) IsMemberAccessible() bool { + return false +} + func (*AddressType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -6526,6 +6578,10 @@ func (*TransactionType) IsComparable() bool { return false } +func (*TransactionType) IsMemberAccessible() bool { + return false +} + func (*TransactionType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -6763,6 +6819,10 @@ func (t *RestrictedType) IsComparable() bool { return false } +func (*RestrictedType) IsMemberAccessible() bool { + return true +} + func (*RestrictedType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateValid } @@ -7038,6 +7098,10 @@ func (*CapabilityType) IsComparable() bool { return false } +func (*CapabilityType) IsMemberAccessible() bool { + return false +} + func (t *CapabilityType) RewriteWithRestrictedTypes() (Type, bool) { if t.BorrowType == nil { return t, false @@ -7591,6 +7655,10 @@ func (*EntitlementType) IsResourceType() bool { return false } +func (*EntitlementType) IsMemberAccessible() bool { + return false +} + func (*EntitlementType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateDirectEntitlementTypeAnnotation } @@ -7721,6 +7789,10 @@ func (*EntitlementMapType) IsResourceType() bool { return false } +func (*EntitlementMapType) IsMemberAccessible() bool { + return false +} + func (*EntitlementMapType) TypeAnnotationState() TypeAnnotationState { return TypeAnnotationStateDirectEntitlementTypeAnnotation } diff --git a/runtime/tests/checker/member_test.go b/runtime/tests/checker/member_test.go index 289b6c7026..180ae6bae1 100644 --- a/runtime/tests/checker/member_test.go +++ b/runtime/tests/checker/member_test.go @@ -19,11 +19,13 @@ package checker import ( + "fmt" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" ) @@ -462,7 +464,7 @@ func TestCheckMemberAccess(t *testing.T) { require.NoError(t, err) }) - t.Run("composite reference, field", func(t *testing.T) { + t.Run("composite reference, array field", func(t *testing.T) { t.Parallel() _, err := ParseAndCheck(t, ` @@ -713,4 +715,54 @@ func TestCheckMemberAccess(t *testing.T) { assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) }) + + t.Run("all member types", func(t *testing.T) { + t.Parallel() + + test := func(tt *testing.T, typeName string) { + code := fmt.Sprintf(` + struct Foo { + var a: %[1]s? + + init() { + self.a = nil + } + } + + struct Bar {} + + struct interface I {} + + fun test() { + let foo = Foo() + let fooRef = &foo as &Foo + var a: &%[1]s? = fooRef.a + }`, + + typeName, + ) + + _, err := ParseAndCheck(t, code) + require.NoError(t, err) + } + + types := []string{ + "Bar", + "{I}", + "AnyStruct", + "Block", + } + + // Test all built-in composite types + for i := interpreter.PrimitiveStaticTypeAuthAccount; i < interpreter.PrimitiveStaticType_Count; i++ { + semaType := i.SemaType() + types = append(types, semaType.QualifiedString()) + } + + for _, typeName := range types { + t.Run(typeName, func(t *testing.T) { + test(t, typeName) + }) + } + }) } diff --git a/runtime/tests/interpreter/member_test.go b/runtime/tests/interpreter/member_test.go index 68d6f491e6..87885d0741 100644 --- a/runtime/tests/interpreter/member_test.go +++ b/runtime/tests/interpreter/member_test.go @@ -19,6 +19,7 @@ package interpreter_test import ( + "fmt" "testing" "github.com/stretchr/testify/require" @@ -1024,4 +1025,58 @@ func TestInterpretMemberAccess(t *testing.T) { _, err := inter.Invoke("test") require.NoError(t, err) }) + + t.Run("all member types", func(t *testing.T) { + t.Parallel() + + test := func(tt *testing.T, typeName string) { + code := fmt.Sprintf(` + struct Foo { + var a: %[1]s? + + init() { + self.a = nil + } + } + + struct Bar {} + + struct interface I {} + + fun test() { + let foo = Foo() + let fooRef = &foo as &Foo + var a: &%[1]s? = fooRef.a + }`, + + typeName, + ) + + inter := parseCheckAndInterpret(t, code) + + _, err := inter.Invoke("test") + require.NoError(t, err) + } + + types := []string{ + "Bar", + "{I}", + "[Int]", + "{Bool: String}", + "AnyStruct", + "Block", + } + + // Test all built-in composite types + for i := interpreter.PrimitiveStaticTypeAuthAccount; i < interpreter.PrimitiveStaticType_Count; i++ { + semaType := i.SemaType() + types = append(types, semaType.QualifiedString()) + } + + for _, typeName := range types { + t.Run(typeName, func(t *testing.T) { + test(t, typeName) + }) + } + }) } From c16e5682fa679469cd52d9331d1f117d1275eed9 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Fri, 23 Jun 2023 15:32:05 -0700 Subject: [PATCH 20/24] Update tests --- runtime/sema/check_member_expression.go | 2 +- .../sema/gen/testdata/comparable.golden.go | 21 +++++------ .../sema/gen/testdata/docstrings.golden.go | 21 +++++------ runtime/sema/gen/testdata/equatable.golden.go | 21 +++++------ .../sema/gen/testdata/exportable.golden.go | 21 +++++------ runtime/sema/gen/testdata/fields.golden.go | 21 +++++------ runtime/sema/gen/testdata/functions.golden.go | 21 +++++------ .../sema/gen/testdata/importable.golden.go | 21 +++++------ .../sema/gen/testdata/member_accessible.cdc | 1 + .../gen/testdata/member_accessible.golden.go | 36 +++++++++++++++++++ .../gen/testdata/simple-resource.golden.go | 21 +++++------ .../sema/gen/testdata/simple-struct.golden.go | 21 +++++------ runtime/sema/gen/testdata/storable.golden.go | 21 +++++------ 13 files changed, 148 insertions(+), 101 deletions(-) create mode 100644 runtime/sema/gen/testdata/member_accessible.cdc create mode 100644 runtime/sema/gen/testdata/member_accessible.golden.go diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index da1f637776..49e5225a0c 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -108,7 +108,7 @@ func (checker *Checker) getReferenceType(typ Type) Type { } func shouldReturnReference(parentType, memberType Type) bool { - if !isReferenceType(parentType) { + if memberType == nil || !isReferenceType(parentType) { return false } diff --git a/runtime/sema/gen/testdata/comparable.golden.go b/runtime/sema/gen/testdata/comparable.golden.go index 4dd82cb6d0..a6947fd5d4 100644 --- a/runtime/sema/gen/testdata/comparable.golden.go +++ b/runtime/sema/gen/testdata/comparable.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: true, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: true, + Exportable: false, + Importable: false, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/docstrings.golden.go b/runtime/sema/gen/testdata/docstrings.golden.go index e27183f0e2..f24cf2a87e 100644 --- a/runtime/sema/gen/testdata/docstrings.golden.go +++ b/runtime/sema/gen/testdata/docstrings.golden.go @@ -104,16 +104,17 @@ Look, I did it ` + "`again`" + `, wowie!! const DocstringsTypeName = "Docstrings" var DocstringsType = &SimpleType{ - Name: DocstringsTypeName, - QualifiedName: DocstringsTypeName, - TypeID: DocstringsTypeName, - tag: DocstringsTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: DocstringsTypeName, + QualifiedName: DocstringsTypeName, + TypeID: DocstringsTypeName, + tag: DocstringsTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } func init() { diff --git a/runtime/sema/gen/testdata/equatable.golden.go b/runtime/sema/gen/testdata/equatable.golden.go index 291dfaadd5..3c91a7b6d4 100644 --- a/runtime/sema/gen/testdata/equatable.golden.go +++ b/runtime/sema/gen/testdata/equatable.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: true, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: true, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/exportable.golden.go b/runtime/sema/gen/testdata/exportable.golden.go index c184ae6b2b..d59f329c5a 100644 --- a/runtime/sema/gen/testdata/exportable.golden.go +++ b/runtime/sema/gen/testdata/exportable.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: true, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: true, + Importable: false, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/fields.golden.go b/runtime/sema/gen/testdata/fields.golden.go index 6a55805536..3aaba29c4f 100644 --- a/runtime/sema/gen/testdata/fields.golden.go +++ b/runtime/sema/gen/testdata/fields.golden.go @@ -151,16 +151,17 @@ This is a test restricted type (without restrictions) field. const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } func init() { diff --git a/runtime/sema/gen/testdata/functions.golden.go b/runtime/sema/gen/testdata/functions.golden.go index 0e108fb480..3656499db0 100644 --- a/runtime/sema/gen/testdata/functions.golden.go +++ b/runtime/sema/gen/testdata/functions.golden.go @@ -176,16 +176,17 @@ This is a function with 'view' modifier const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } func init() { diff --git a/runtime/sema/gen/testdata/importable.golden.go b/runtime/sema/gen/testdata/importable.golden.go index 42778d5dcb..d907863f94 100644 --- a/runtime/sema/gen/testdata/importable.golden.go +++ b/runtime/sema/gen/testdata/importable.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: true, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: true, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/member_accessible.cdc b/runtime/sema/gen/testdata/member_accessible.cdc new file mode 100644 index 0000000000..581047e037 --- /dev/null +++ b/runtime/sema/gen/testdata/member_accessible.cdc @@ -0,0 +1 @@ +access(all) struct Test: MemberAccessible {} diff --git a/runtime/sema/gen/testdata/member_accessible.golden.go b/runtime/sema/gen/testdata/member_accessible.golden.go new file mode 100644 index 0000000000..83dd24caba --- /dev/null +++ b/runtime/sema/gen/testdata/member_accessible.golden.go @@ -0,0 +1,36 @@ +// Code generated from testdata/member_accessible.cdc. DO NOT EDIT. +/* + * Cadence - The resource-oriented smart contract programming language + * + * Copyright Dapper Labs, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package sema + +const TestTypeName = "Test" + +var TestType = &SimpleType{ + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: true, +} diff --git a/runtime/sema/gen/testdata/simple-resource.golden.go b/runtime/sema/gen/testdata/simple-resource.golden.go index 4028abf652..34ed967b31 100644 --- a/runtime/sema/gen/testdata/simple-resource.golden.go +++ b/runtime/sema/gen/testdata/simple-resource.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: true, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: true, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/simple-struct.golden.go b/runtime/sema/gen/testdata/simple-struct.golden.go index 04cf626437..6e2db9e165 100644 --- a/runtime/sema/gen/testdata/simple-struct.golden.go +++ b/runtime/sema/gen/testdata/simple-struct.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } diff --git a/runtime/sema/gen/testdata/storable.golden.go b/runtime/sema/gen/testdata/storable.golden.go index adcaf51f6c..ff077d2add 100644 --- a/runtime/sema/gen/testdata/storable.golden.go +++ b/runtime/sema/gen/testdata/storable.golden.go @@ -22,14 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: true, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: true, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + MemberAccessible: false, } From 84e7e0fd759fed08fdbd04e0b0be0b8d722b8eed Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 4 Jul 2023 07:51:16 -0700 Subject: [PATCH 21/24] Rename MemberInfo to MemberAccessInfo --- runtime/interpreter/interpreter_expression.go | 8 +-- runtime/sema/check_member_expression.go | 6 +- runtime/sema/elaboration.go | 70 +++++++++---------- 3 files changed, 42 insertions(+), 42 deletions(-) diff --git a/runtime/interpreter/interpreter_expression.go b/runtime/interpreter/interpreter_expression.go index 96ebed3636..37753b5647 100644 --- a/runtime/interpreter/interpreter_expression.go +++ b/runtime/interpreter/interpreter_expression.go @@ -172,11 +172,11 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a isNestedResourceMove := interpreter.Program.Elaboration.IsNestedResourceMoveExpression(memberExpression) - memberInfo, ok := interpreter.Program.Elaboration.MemberExpressionMemberInfo(memberExpression) + memberAccessInfo, ok := interpreter.Program.Elaboration.MemberExpressionMemberAccessInfo(memberExpression) if !ok { panic(errors.NewUnreachableError()) } - memberType := memberInfo.Member.TypeAnnotation.Type + memberType := memberAccessInfo.Member.TypeAnnotation.Type return getterSetter{ target: target, @@ -223,7 +223,7 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a // Return a reference, if the member is accessed via a reference. // This is pre-computed at the checker. - if memberInfo.ReturnReference { + if memberAccessInfo.ReturnReference { // Get a reference to the value resultValue = interpreter.getReferenceValue(resultValue, memberType) } @@ -278,7 +278,7 @@ func (interpreter *Interpreter) checkMemberAccess( target Value, locationRange LocationRange, ) { - memberInfo, _ := interpreter.Program.Elaboration.MemberExpressionMemberInfo(memberExpression) + memberInfo, _ := interpreter.Program.Elaboration.MemberExpressionMemberAccessInfo(memberExpression) expectedType := memberInfo.AccessedType switch expectedType := expectedType.(type) { diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index 49e5225a0c..ac23882aca 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -122,7 +122,7 @@ func isReferenceType(typ Type) bool { } func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedType Type, resultingType Type, member *Member, isOptional bool) { - memberInfo, ok := checker.Elaboration.MemberExpressionMemberInfo(expression) + memberInfo, ok := checker.Elaboration.MemberExpressionMemberAccessInfo(expression) if ok { return memberInfo.AccessedType, memberInfo.ResultingType, memberInfo.Member, memberInfo.IsOptional } @@ -130,9 +130,9 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT returnReference := false defer func() { - checker.Elaboration.SetMemberExpressionMemberInfo( + checker.Elaboration.SetMemberExpressionMemberAccessInfo( expression, - MemberInfo{ + MemberAccessInfo{ AccessedType: accessedType, ResultingType: resultingType, Member: member, diff --git a/runtime/sema/elaboration.go b/runtime/sema/elaboration.go index af2224ce45..770e32e0a0 100644 --- a/runtime/sema/elaboration.go +++ b/runtime/sema/elaboration.go @@ -25,7 +25,7 @@ import ( "github.com/onflow/cadence/runtime/common" ) -type MemberInfo struct { +type MemberAccessInfo struct { AccessedType Type ResultingType Type Member *Member @@ -110,33 +110,33 @@ type ExpressionTypes struct { } type Elaboration struct { - fixedPointExpressionTypes map[*ast.FixedPointExpression]Type - interfaceTypeDeclarations map[*InterfaceType]*ast.InterfaceDeclaration - entitlementTypeDeclarations map[*EntitlementType]*ast.EntitlementDeclaration - entitlementMapTypeDeclarations map[*EntitlementMapType]*ast.EntitlementMappingDeclaration - swapStatementTypes map[*ast.SwapStatement]SwapStatementTypes - assignmentStatementTypes map[*ast.AssignmentStatement]AssignmentStatementTypes - compositeDeclarationTypes map[ast.CompositeLikeDeclaration]*CompositeType - compositeTypeDeclarations map[*CompositeType]ast.CompositeLikeDeclaration - interfaceDeclarationTypes map[*ast.InterfaceDeclaration]*InterfaceType - entitlementDeclarationTypes map[*ast.EntitlementDeclaration]*EntitlementType - entitlementMapDeclarationTypes map[*ast.EntitlementMappingDeclaration]*EntitlementMapType - transactionDeclarationTypes map[*ast.TransactionDeclaration]*TransactionType - constructorFunctionTypes map[*ast.SpecialFunctionDeclaration]*FunctionType - functionExpressionFunctionTypes map[*ast.FunctionExpression]*FunctionType - invocationExpressionTypes map[*ast.InvocationExpression]InvocationExpressionTypes - castingExpressionTypes map[*ast.CastingExpression]CastingExpressionTypes - lock *sync.RWMutex - binaryExpressionTypes map[*ast.BinaryExpression]BinaryExpressionTypes - memberExpressionMemberInfos map[*ast.MemberExpression]MemberInfo - memberExpressionExpectedTypes map[*ast.MemberExpression]Type - arrayExpressionTypes map[*ast.ArrayExpression]ArrayExpressionTypes - dictionaryExpressionTypes map[*ast.DictionaryExpression]DictionaryExpressionTypes - integerExpressionTypes map[*ast.IntegerExpression]Type - stringExpressionTypes map[*ast.StringExpression]Type - returnStatementTypes map[*ast.ReturnStatement]ReturnStatementTypes - functionDeclarationFunctionTypes map[*ast.FunctionDeclaration]*FunctionType - variableDeclarationTypes map[*ast.VariableDeclaration]VariableDeclarationTypes + fixedPointExpressionTypes map[*ast.FixedPointExpression]Type + interfaceTypeDeclarations map[*InterfaceType]*ast.InterfaceDeclaration + entitlementTypeDeclarations map[*EntitlementType]*ast.EntitlementDeclaration + entitlementMapTypeDeclarations map[*EntitlementMapType]*ast.EntitlementMappingDeclaration + swapStatementTypes map[*ast.SwapStatement]SwapStatementTypes + assignmentStatementTypes map[*ast.AssignmentStatement]AssignmentStatementTypes + compositeDeclarationTypes map[ast.CompositeLikeDeclaration]*CompositeType + compositeTypeDeclarations map[*CompositeType]ast.CompositeLikeDeclaration + interfaceDeclarationTypes map[*ast.InterfaceDeclaration]*InterfaceType + entitlementDeclarationTypes map[*ast.EntitlementDeclaration]*EntitlementType + entitlementMapDeclarationTypes map[*ast.EntitlementMappingDeclaration]*EntitlementMapType + transactionDeclarationTypes map[*ast.TransactionDeclaration]*TransactionType + constructorFunctionTypes map[*ast.SpecialFunctionDeclaration]*FunctionType + functionExpressionFunctionTypes map[*ast.FunctionExpression]*FunctionType + invocationExpressionTypes map[*ast.InvocationExpression]InvocationExpressionTypes + castingExpressionTypes map[*ast.CastingExpression]CastingExpressionTypes + lock *sync.RWMutex + binaryExpressionTypes map[*ast.BinaryExpression]BinaryExpressionTypes + memberExpressionMemberAccessInfos map[*ast.MemberExpression]MemberAccessInfo + memberExpressionExpectedTypes map[*ast.MemberExpression]Type + arrayExpressionTypes map[*ast.ArrayExpression]ArrayExpressionTypes + dictionaryExpressionTypes map[*ast.DictionaryExpression]DictionaryExpressionTypes + integerExpressionTypes map[*ast.IntegerExpression]Type + stringExpressionTypes map[*ast.StringExpression]Type + returnStatementTypes map[*ast.ReturnStatement]ReturnStatementTypes + functionDeclarationFunctionTypes map[*ast.FunctionDeclaration]*FunctionType + variableDeclarationTypes map[*ast.VariableDeclaration]VariableDeclarationTypes // nestedResourceMoveExpressions indicates the index or member expression // is implicitly moving a resource out of the container, e.g. in a shift or swap statement. nestedResourceMoveExpressions map[ast.Expression]struct{} @@ -637,20 +637,20 @@ func (e *Elaboration) SetIntegerExpressionType(expression *ast.IntegerExpression e.integerExpressionTypes[expression] = actualType } -func (e *Elaboration) MemberExpressionMemberInfo(expression *ast.MemberExpression) (memberInfo MemberInfo, ok bool) { - if e.memberExpressionMemberInfos == nil { +func (e *Elaboration) MemberExpressionMemberAccessInfo(expression *ast.MemberExpression) (memberInfo MemberAccessInfo, ok bool) { + if e.memberExpressionMemberAccessInfos == nil { ok = false return } - memberInfo, ok = e.memberExpressionMemberInfos[expression] + memberInfo, ok = e.memberExpressionMemberAccessInfos[expression] return } -func (e *Elaboration) SetMemberExpressionMemberInfo(expression *ast.MemberExpression, memberInfo MemberInfo) { - if e.memberExpressionMemberInfos == nil { - e.memberExpressionMemberInfos = map[*ast.MemberExpression]MemberInfo{} +func (e *Elaboration) SetMemberExpressionMemberAccessInfo(expression *ast.MemberExpression, memberAccessInfo MemberAccessInfo) { + if e.memberExpressionMemberAccessInfos == nil { + e.memberExpressionMemberAccessInfos = map[*ast.MemberExpression]MemberAccessInfo{} } - e.memberExpressionMemberInfos[expression] = memberInfo + e.memberExpressionMemberAccessInfos[expression] = memberAccessInfo } func (e *Elaboration) MemberExpressionExpectedType(expression *ast.MemberExpression) Type { From b72a3b1f04e100888d52f2a59c8a6b47cc95b312 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 4 Jul 2023 07:57:28 -0700 Subject: [PATCH 22/24] Update tests --- runtime/resourcedictionary_test.go | 6 ++---- runtime/tests/checker/swap_test.go | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/runtime/resourcedictionary_test.go b/runtime/resourcedictionary_test.go index 492ff99612..b511bceb9c 100644 --- a/runtime/resourcedictionary_test.go +++ b/runtime/resourcedictionary_test.go @@ -252,12 +252,11 @@ func TestRuntimeResourceDictionaryValues(t *testing.T) { transaction { prepare(signer: AuthAccount) { - let c <- signer.load<@Test.C>(from: /storage/c)! + let c = signer.borrow<&Test.C>(from: /storage/c)! log(c.rs["b"]?.value) destroy c.remove("b") c.forceInsert("b", <- Test.createR(4)) log(c.rs["b"]?.value) - signer.save(<-c, to: /storage/c) } } `) @@ -293,11 +292,10 @@ func TestRuntimeResourceDictionaryValues(t *testing.T) { transaction { prepare(signer: AuthAccount) { - let c <- signer.load<@Test.C>(from: /storage/c)! + let c = signer.borrow<&Test.C>(from: /storage/c)! log(c.rs["b"]?.value) destroy c.remove("b") log(c.rs["b"]?.value) - signer.save(<-c, to: /storage/c) } } `) diff --git a/runtime/tests/checker/swap_test.go b/runtime/tests/checker/swap_test.go index 3e35af8884..ccb93a38ad 100644 --- a/runtime/tests/checker/swap_test.go +++ b/runtime/tests/checker/swap_test.go @@ -108,7 +108,7 @@ func TestCheckInvalidTypesSwap(t *testing.T) { errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) - assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) } func TestCheckInvalidTypesSwap2(t *testing.T) { @@ -126,7 +126,7 @@ func TestCheckInvalidTypesSwap2(t *testing.T) { errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) - assert.IsType(t, &sema.TypeMismatchError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) } func TestCheckInvalidSwapTargetExpressionLeft(t *testing.T) { From 26149a068c5f0788114c8432e0a7ca0e5b04701d Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Tue, 4 Jul 2023 08:39:23 -0700 Subject: [PATCH 23/24] Fix tidy --- runtime/sema/authaccount.cdc | 2 +- runtime/sema/character.gen.go | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/runtime/sema/authaccount.cdc b/runtime/sema/authaccount.cdc index 391263a5f4..e987a4094b 100644 --- a/runtime/sema/authaccount.cdc +++ b/runtime/sema/authaccount.cdc @@ -1,5 +1,5 @@ -access(all) struct AuthAccount: MemberAccessible { +access(all) struct AuthAccount { /// The address of the account. access(all) let address: Address diff --git a/runtime/sema/character.gen.go b/runtime/sema/character.gen.go index c2e2370a15..b2defc9f72 100644 --- a/runtime/sema/character.gen.go +++ b/runtime/sema/character.gen.go @@ -36,16 +36,17 @@ Returns this character as a String const CharacterTypeName = "Character" var CharacterType = &SimpleType{ - Name: CharacterTypeName, - QualifiedName: CharacterTypeName, - TypeID: CharacterTypeName, - tag: CharacterTypeTag, - IsResource: false, - Storable: true, - Equatable: true, - Comparable: true, - Exportable: true, - Importable: true, + Name: CharacterTypeName, + QualifiedName: CharacterTypeName, + TypeID: CharacterTypeName, + tag: CharacterTypeTag, + IsResource: false, + Storable: true, + Equatable: true, + Comparable: true, + Exportable: true, + Importable: true, + MemberAccessible: false, } func init() { From e47f4517f08f5cb6ea8bf24f8ec8dc88fbc081ed Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 5 Jul 2023 16:20:22 -0700 Subject: [PATCH 24/24] Rename 'MemberAccessible' to 'ContainFieldsOrElements' --- .../sema/account_capability_controller.cdc | 2 +- .../sema/account_capability_controller.gen.go | 22 +++---- runtime/sema/anyresource_type.go | 6 +- runtime/sema/anystruct_type.go | 4 +- runtime/sema/block.cdc | 2 +- runtime/sema/block.gen.go | 22 +++---- runtime/sema/character.gen.go | 22 +++---- runtime/sema/check_member_expression.go | 2 +- runtime/sema/deployedcontract.cdc | 2 +- runtime/sema/deployedcontract.gen.go | 22 +++---- runtime/sema/gen/main.go | 6 +- .../sema/gen/testdata/comparable.golden.go | 22 +++---- .../sema/gen/testdata/docstrings.golden.go | 22 +++---- runtime/sema/gen/testdata/equatable.golden.go | 22 +++---- .../sema/gen/testdata/exportable.golden.go | 22 +++---- runtime/sema/gen/testdata/fields.golden.go | 22 +++---- runtime/sema/gen/testdata/functions.golden.go | 22 +++---- .../sema/gen/testdata/importable.golden.go | 22 +++---- .../sema/gen/testdata/member_accessible.cdc | 2 +- .../gen/testdata/member_accessible.golden.go | 22 +++---- .../gen/testdata/simple-resource.golden.go | 22 +++---- .../sema/gen/testdata/simple-struct.golden.go | 22 +++---- runtime/sema/gen/testdata/storable.golden.go | 22 +++---- runtime/sema/simple_type.go | 6 +- .../sema/storage_capability_controller.cdc | 2 +- .../sema/storage_capability_controller.gen.go | 22 +++---- runtime/sema/type.go | 59 ++++++++++++------- 27 files changed, 231 insertions(+), 214 deletions(-) diff --git a/runtime/sema/account_capability_controller.cdc b/runtime/sema/account_capability_controller.cdc index 2d3e9f9b64..3442c24560 100644 --- a/runtime/sema/account_capability_controller.cdc +++ b/runtime/sema/account_capability_controller.cdc @@ -1,4 +1,4 @@ -access(all) struct AccountCapabilityController: MemberAccessible { +access(all) struct AccountCapabilityController: ContainFields { /// An arbitrary "tag" for the controller. /// For example, it could be used to describe the purpose of the capability. diff --git a/runtime/sema/account_capability_controller.gen.go b/runtime/sema/account_capability_controller.gen.go index a71550bd63..e66d6f8ca1 100644 --- a/runtime/sema/account_capability_controller.gen.go +++ b/runtime/sema/account_capability_controller.gen.go @@ -91,17 +91,17 @@ Borrowing from the controlled capability or its copies will return nil. const AccountCapabilityControllerTypeName = "AccountCapabilityController" var AccountCapabilityControllerType = &SimpleType{ - Name: AccountCapabilityControllerTypeName, - QualifiedName: AccountCapabilityControllerTypeName, - TypeID: AccountCapabilityControllerTypeName, - tag: AccountCapabilityControllerTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: true, + Name: AccountCapabilityControllerTypeName, + QualifiedName: AccountCapabilityControllerTypeName, + TypeID: AccountCapabilityControllerTypeName, + tag: AccountCapabilityControllerTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: true, } func init() { diff --git a/runtime/sema/anyresource_type.go b/runtime/sema/anyresource_type.go index 19e41d3133..b19d72040f 100644 --- a/runtime/sema/anyresource_type.go +++ b/runtime/sema/anyresource_type.go @@ -30,7 +30,7 @@ var AnyResourceType = &SimpleType{ Equatable: false, Comparable: false, // The actual returnability of a value is checked at run-time - Exportable: true, - Importable: false, - MemberAccessible: true, + Exportable: true, + Importable: false, + ContainFields: true, } diff --git a/runtime/sema/anystruct_type.go b/runtime/sema/anystruct_type.go index 339d81df20..edbc5e8220 100644 --- a/runtime/sema/anystruct_type.go +++ b/runtime/sema/anystruct_type.go @@ -31,8 +31,8 @@ var AnyStructType = &SimpleType{ Comparable: false, Exportable: true, // The actual importability is checked at runtime - Importable: true, - MemberAccessible: true, + Importable: true, + ContainFields: true, } var AnyStructTypeAnnotation = NewTypeAnnotation(AnyStructType) diff --git a/runtime/sema/block.cdc b/runtime/sema/block.cdc index f2b25075b7..97515f8f3c 100644 --- a/runtime/sema/block.cdc +++ b/runtime/sema/block.cdc @@ -1,5 +1,5 @@ -access(all) struct Block: MemberAccessible { +access(all) struct Block: ContainFields { /// The height of the block. /// diff --git a/runtime/sema/block.gen.go b/runtime/sema/block.gen.go index f74a58e29c..3bdd5a712e 100644 --- a/runtime/sema/block.gen.go +++ b/runtime/sema/block.gen.go @@ -66,17 +66,17 @@ It is essentially the hash of the block const BlockTypeName = "Block" var BlockType = &SimpleType{ - Name: BlockTypeName, - QualifiedName: BlockTypeName, - TypeID: BlockTypeName, - tag: BlockTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: true, + Name: BlockTypeName, + QualifiedName: BlockTypeName, + TypeID: BlockTypeName, + tag: BlockTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: true, } func init() { diff --git a/runtime/sema/character.gen.go b/runtime/sema/character.gen.go index b2defc9f72..a65f606a44 100644 --- a/runtime/sema/character.gen.go +++ b/runtime/sema/character.gen.go @@ -36,17 +36,17 @@ Returns this character as a String const CharacterTypeName = "Character" var CharacterType = &SimpleType{ - Name: CharacterTypeName, - QualifiedName: CharacterTypeName, - TypeID: CharacterTypeName, - tag: CharacterTypeTag, - IsResource: false, - Storable: true, - Equatable: true, - Comparable: true, - Exportable: true, - Importable: true, - MemberAccessible: false, + Name: CharacterTypeName, + QualifiedName: CharacterTypeName, + TypeID: CharacterTypeName, + tag: CharacterTypeTag, + IsResource: false, + Storable: true, + Equatable: true, + Comparable: true, + Exportable: true, + Importable: true, + ContainFields: false, } func init() { diff --git a/runtime/sema/check_member_expression.go b/runtime/sema/check_member_expression.go index ac23882aca..66d9a2cc6d 100644 --- a/runtime/sema/check_member_expression.go +++ b/runtime/sema/check_member_expression.go @@ -112,7 +112,7 @@ func shouldReturnReference(parentType, memberType Type) bool { return false } - return memberType.IsMemberAccessible() + return memberType.ContainFieldsOrElements() } func isReferenceType(typ Type) bool { diff --git a/runtime/sema/deployedcontract.cdc b/runtime/sema/deployedcontract.cdc index f3e824243a..11e611bfbc 100644 --- a/runtime/sema/deployedcontract.cdc +++ b/runtime/sema/deployedcontract.cdc @@ -1,5 +1,5 @@ -access(all) struct DeployedContract: MemberAccessible { +access(all) struct DeployedContract: ContainFields { /// The address of the account where the contract is deployed at. access(all) let address: Address diff --git a/runtime/sema/deployedcontract.gen.go b/runtime/sema/deployedcontract.gen.go index f473d72c81..621b08d473 100644 --- a/runtime/sema/deployedcontract.gen.go +++ b/runtime/sema/deployedcontract.gen.go @@ -74,17 +74,17 @@ then ` + "`.publicTypes()`" + ` will return an array equivalent to the expressio const DeployedContractTypeName = "DeployedContract" var DeployedContractType = &SimpleType{ - Name: DeployedContractTypeName, - QualifiedName: DeployedContractTypeName, - TypeID: DeployedContractTypeName, - tag: DeployedContractTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: true, + Name: DeployedContractTypeName, + QualifiedName: DeployedContractTypeName, + TypeID: DeployedContractTypeName, + tag: DeployedContractTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: true, } func init() { diff --git a/runtime/sema/gen/main.go b/runtime/sema/gen/main.go index 5c6d2fdf11..8337722b3c 100644 --- a/runtime/sema/gen/main.go +++ b/runtime/sema/gen/main.go @@ -425,10 +425,10 @@ func (g *generator) VisitCompositeDeclaration(decl *ast.CompositeDeclaration) (_ case "Importable": typeDecl.importable = true - case "MemberAccessible": + case "ContainFields": if !canGenerateSimpleType { panic(fmt.Errorf( - "composite types cannot be explicitly marked as member accessible: %s", + "composite types cannot be explicitly marked as having fields: %s", g.currentTypeID(), )) } @@ -1168,7 +1168,7 @@ func simpleTypeLiteral(ty *typeDecl) dst.Expr { goKeyValue("Comparable", goBoolLit(ty.comparable)), goKeyValue("Exportable", goBoolLit(ty.exportable)), goKeyValue("Importable", goBoolLit(ty.importable)), - goKeyValue("MemberAccessible", goBoolLit(ty.memberAccessible)), + goKeyValue("ContainFields", goBoolLit(ty.memberAccessible)), } return &dst.UnaryExpr{ diff --git a/runtime/sema/gen/testdata/comparable.golden.go b/runtime/sema/gen/testdata/comparable.golden.go index a6947fd5d4..610a531517 100644 --- a/runtime/sema/gen/testdata/comparable.golden.go +++ b/runtime/sema/gen/testdata/comparable.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: true, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: true, + Exportable: false, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/docstrings.golden.go b/runtime/sema/gen/testdata/docstrings.golden.go index f24cf2a87e..846a220c43 100644 --- a/runtime/sema/gen/testdata/docstrings.golden.go +++ b/runtime/sema/gen/testdata/docstrings.golden.go @@ -104,17 +104,17 @@ Look, I did it ` + "`again`" + `, wowie!! const DocstringsTypeName = "Docstrings" var DocstringsType = &SimpleType{ - Name: DocstringsTypeName, - QualifiedName: DocstringsTypeName, - TypeID: DocstringsTypeName, - tag: DocstringsTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: DocstringsTypeName, + QualifiedName: DocstringsTypeName, + TypeID: DocstringsTypeName, + tag: DocstringsTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } func init() { diff --git a/runtime/sema/gen/testdata/equatable.golden.go b/runtime/sema/gen/testdata/equatable.golden.go index 3c91a7b6d4..82320957ee 100644 --- a/runtime/sema/gen/testdata/equatable.golden.go +++ b/runtime/sema/gen/testdata/equatable.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: true, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: true, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/exportable.golden.go b/runtime/sema/gen/testdata/exportable.golden.go index d59f329c5a..db6afd0593 100644 --- a/runtime/sema/gen/testdata/exportable.golden.go +++ b/runtime/sema/gen/testdata/exportable.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: true, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: true, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/fields.golden.go b/runtime/sema/gen/testdata/fields.golden.go index 3aaba29c4f..1e8a8676d2 100644 --- a/runtime/sema/gen/testdata/fields.golden.go +++ b/runtime/sema/gen/testdata/fields.golden.go @@ -151,17 +151,17 @@ This is a test restricted type (without restrictions) field. const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } func init() { diff --git a/runtime/sema/gen/testdata/functions.golden.go b/runtime/sema/gen/testdata/functions.golden.go index 3656499db0..fafd4cfaf4 100644 --- a/runtime/sema/gen/testdata/functions.golden.go +++ b/runtime/sema/gen/testdata/functions.golden.go @@ -176,17 +176,17 @@ This is a function with 'view' modifier const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } func init() { diff --git a/runtime/sema/gen/testdata/importable.golden.go b/runtime/sema/gen/testdata/importable.golden.go index d907863f94..39496013d4 100644 --- a/runtime/sema/gen/testdata/importable.golden.go +++ b/runtime/sema/gen/testdata/importable.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: true, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: true, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/member_accessible.cdc b/runtime/sema/gen/testdata/member_accessible.cdc index 581047e037..1c83dfc60a 100644 --- a/runtime/sema/gen/testdata/member_accessible.cdc +++ b/runtime/sema/gen/testdata/member_accessible.cdc @@ -1 +1 @@ -access(all) struct Test: MemberAccessible {} +access(all) struct Test: ContainFields {} diff --git a/runtime/sema/gen/testdata/member_accessible.golden.go b/runtime/sema/gen/testdata/member_accessible.golden.go index 83dd24caba..d3553ebafd 100644 --- a/runtime/sema/gen/testdata/member_accessible.golden.go +++ b/runtime/sema/gen/testdata/member_accessible.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: true, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: true, } diff --git a/runtime/sema/gen/testdata/simple-resource.golden.go b/runtime/sema/gen/testdata/simple-resource.golden.go index 34ed967b31..53ee6c24f7 100644 --- a/runtime/sema/gen/testdata/simple-resource.golden.go +++ b/runtime/sema/gen/testdata/simple-resource.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: true, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: true, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/simple-struct.golden.go b/runtime/sema/gen/testdata/simple-struct.golden.go index 6e2db9e165..0429d008f3 100644 --- a/runtime/sema/gen/testdata/simple-struct.golden.go +++ b/runtime/sema/gen/testdata/simple-struct.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/gen/testdata/storable.golden.go b/runtime/sema/gen/testdata/storable.golden.go index ff077d2add..c9c5526991 100644 --- a/runtime/sema/gen/testdata/storable.golden.go +++ b/runtime/sema/gen/testdata/storable.golden.go @@ -22,15 +22,15 @@ package sema const TestTypeName = "Test" var TestType = &SimpleType{ - Name: TestTypeName, - QualifiedName: TestTypeName, - TypeID: TestTypeName, - tag: TestTypeTag, - IsResource: false, - Storable: true, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: false, + Name: TestTypeName, + QualifiedName: TestTypeName, + TypeID: TestTypeName, + tag: TestTypeTag, + IsResource: false, + Storable: true, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: false, } diff --git a/runtime/sema/simple_type.go b/runtime/sema/simple_type.go index cf411991e8..5a46b934c6 100644 --- a/runtime/sema/simple_type.go +++ b/runtime/sema/simple_type.go @@ -50,7 +50,7 @@ type SimpleType struct { Comparable bool Storable bool IsResource bool - MemberAccessible bool + ContainFields bool } var _ Type = &SimpleType{} @@ -107,8 +107,8 @@ func (t *SimpleType) IsImportable(_ map[*Member]bool) bool { return t.Importable } -func (t *SimpleType) IsMemberAccessible() bool { - return t.MemberAccessible +func (t *SimpleType) ContainFieldsOrElements() bool { + return t.ContainFields } func (*SimpleType) TypeAnnotationState() TypeAnnotationState { diff --git a/runtime/sema/storage_capability_controller.cdc b/runtime/sema/storage_capability_controller.cdc index 0f9ba83dba..665c4a73c4 100644 --- a/runtime/sema/storage_capability_controller.cdc +++ b/runtime/sema/storage_capability_controller.cdc @@ -1,4 +1,4 @@ -access(all) struct StorageCapabilityController: MemberAccessible { +access(all) struct StorageCapabilityController: ContainFields { /// An arbitrary "tag" for the controller. /// For example, it could be used to describe the purpose of the capability. diff --git a/runtime/sema/storage_capability_controller.gen.go b/runtime/sema/storage_capability_controller.gen.go index ec12b21b75..801ae613b9 100644 --- a/runtime/sema/storage_capability_controller.gen.go +++ b/runtime/sema/storage_capability_controller.gen.go @@ -123,17 +123,17 @@ The path may be different or the same as the current path. const StorageCapabilityControllerTypeName = "StorageCapabilityController" var StorageCapabilityControllerType = &SimpleType{ - Name: StorageCapabilityControllerTypeName, - QualifiedName: StorageCapabilityControllerTypeName, - TypeID: StorageCapabilityControllerTypeName, - tag: StorageCapabilityControllerTypeTag, - IsResource: false, - Storable: false, - Equatable: false, - Comparable: false, - Exportable: false, - Importable: false, - MemberAccessible: true, + Name: StorageCapabilityControllerTypeName, + QualifiedName: StorageCapabilityControllerTypeName, + TypeID: StorageCapabilityControllerTypeName, + tag: StorageCapabilityControllerTypeTag, + IsResource: false, + Storable: false, + Equatable: false, + Comparable: false, + Exportable: false, + Importable: false, + ContainFields: true, } func init() { diff --git a/runtime/sema/type.go b/runtime/sema/type.go index 7613abb3af..0229589772 100644 --- a/runtime/sema/type.go +++ b/runtime/sema/type.go @@ -135,9 +135,26 @@ type Type interface { // IsComparable returns true if values of the type can be compared IsComparable() bool - // IsMemberAccessible returns true if value of the type can have members values. - // Examples are composite types, restricted types, arrays, dictionaries, etc. - IsMemberAccessible() bool + // ContainFieldsOrElements returns true if value of the type can have nested values (fields or elements). + // This notion is to indicate that a type can be used to access its nested values using + // either index-expression or member-expression. e.g. `foo.bar` or `foo[bar]`. + // This is used to determine if a field/element of this type should be returning a reference or not. + // + // Only a subset of types has this characteristic. e.g: + // - Composites + // - Interfaces + // - Arrays (Variable/Constant sized) + // - Dictionaries + // - Restricted types + // - Optionals of the above. + // - Then there are also built-in simple types, like StorageCapabilityControllerType, BlockType, etc. + // where the type is implemented as a simple type, but they also have fields. + // + // This is different from the existing `ValueIndexableType` in the sense that it is also implemented by simple types + // but not all simple types are indexable. + // On the other-hand, some indexable types (e.g. String) shouldn't be treated/returned as references. + // + ContainFieldsOrElements() bool TypeAnnotationState() TypeAnnotationState RewriteWithRestrictedTypes() (result Type, rewritten bool) @@ -642,8 +659,8 @@ func (*OptionalType) IsComparable() bool { return false } -func (t *OptionalType) IsMemberAccessible() bool { - return t.Type.IsMemberAccessible() +func (t *OptionalType) ContainFieldsOrElements() bool { + return t.Type.ContainFieldsOrElements() } func (t *OptionalType) TypeAnnotationState() TypeAnnotationState { @@ -851,7 +868,7 @@ func (*GenericType) IsComparable() bool { return false } -func (t *GenericType) IsMemberAccessible() bool { +func (t *GenericType) ContainFieldsOrElements() bool { return false } @@ -1154,7 +1171,7 @@ func (t *NumericType) IsComparable() bool { return !t.IsSuperType() } -func (t *NumericType) IsMemberAccessible() bool { +func (t *NumericType) ContainFieldsOrElements() bool { return false } @@ -1358,7 +1375,7 @@ func (t *FixedPointNumericType) IsComparable() bool { return !t.IsSuperType() } -func (t *FixedPointNumericType) IsMemberAccessible() bool { +func (t *FixedPointNumericType) ContainFieldsOrElements() bool { return false } @@ -2422,7 +2439,7 @@ func (t *VariableSizedType) IsComparable() bool { return t.Type.IsComparable() } -func (t *VariableSizedType) IsMemberAccessible() bool { +func (t *VariableSizedType) ContainFieldsOrElements() bool { return true } @@ -2576,7 +2593,7 @@ func (t *ConstantSizedType) IsComparable() bool { return t.Type.IsComparable() } -func (t *ConstantSizedType) IsMemberAccessible() bool { +func (t *ConstantSizedType) ContainFieldsOrElements() bool { return true } @@ -3101,7 +3118,7 @@ func (*FunctionType) IsComparable() bool { return false } -func (*FunctionType) IsMemberAccessible() bool { +func (*FunctionType) ContainFieldsOrElements() bool { return false } @@ -4276,7 +4293,7 @@ func (*CompositeType) IsComparable() bool { return false } -func (*CompositeType) IsMemberAccessible() bool { +func (*CompositeType) ContainFieldsOrElements() bool { return true } @@ -4964,7 +4981,7 @@ func (*InterfaceType) IsComparable() bool { return false } -func (*InterfaceType) IsMemberAccessible() bool { +func (*InterfaceType) ContainFieldsOrElements() bool { return true } @@ -5185,7 +5202,7 @@ func (*DictionaryType) IsComparable() bool { return false } -func (*DictionaryType) IsMemberAccessible() bool { +func (*DictionaryType) ContainFieldsOrElements() bool { return true } @@ -5654,7 +5671,7 @@ func (*ReferenceType) IsComparable() bool { return false } -func (*ReferenceType) IsMemberAccessible() bool { +func (*ReferenceType) ContainFieldsOrElements() bool { return false } @@ -5857,7 +5874,7 @@ func (*AddressType) IsComparable() bool { return false } -func (*AddressType) IsMemberAccessible() bool { +func (*AddressType) ContainFieldsOrElements() bool { return false } @@ -6578,7 +6595,7 @@ func (*TransactionType) IsComparable() bool { return false } -func (*TransactionType) IsMemberAccessible() bool { +func (*TransactionType) ContainFieldsOrElements() bool { return false } @@ -6819,7 +6836,7 @@ func (t *RestrictedType) IsComparable() bool { return false } -func (*RestrictedType) IsMemberAccessible() bool { +func (*RestrictedType) ContainFieldsOrElements() bool { return true } @@ -7098,7 +7115,7 @@ func (*CapabilityType) IsComparable() bool { return false } -func (*CapabilityType) IsMemberAccessible() bool { +func (*CapabilityType) ContainFieldsOrElements() bool { return false } @@ -7655,7 +7672,7 @@ func (*EntitlementType) IsResourceType() bool { return false } -func (*EntitlementType) IsMemberAccessible() bool { +func (*EntitlementType) ContainFieldsOrElements() bool { return false } @@ -7789,7 +7806,7 @@ func (*EntitlementMapType) IsResourceType() bool { return false } -func (*EntitlementMapType) IsMemberAccessible() bool { +func (*EntitlementMapType) ContainFieldsOrElements() bool { return false }