Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change member access semantics #2573

Merged
merged 27 commits into from
Jul 6, 2023
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
9743293
Change member access result type to a reference
SupunS Jun 9, 2023
ece3d8f
Fix attachment 'self' access
SupunS Jun 9, 2023
ccc053e
Update interpreter member access
SupunS Jun 12, 2023
e26ff54
Update tests
SupunS Jun 13, 2023
ab854e2
Fix more tests
SupunS Jun 13, 2023
85d294a
Add member-acess expr to static reference validity checking
SupunS Jun 14, 2023
0421eb0
Add test for static attachment invalidation
SupunS Jun 14, 2023
b965147
Refactor code
SupunS Jun 14, 2023
aabca06
Add more tests
SupunS Jun 14, 2023
fc5ea48
Merge branch 'feature/entitlements' of https://github.com/onflow/cade…
SupunS Jun 15, 2023
04e7ac8
Update to match entitlements
SupunS Jun 15, 2023
5c8ea34
Re-use pre-computed results from checker
SupunS Jun 19, 2023
eca92df
Improve tests
SupunS Jun 19, 2023
498c63a
Add test for index expression resource loss
SupunS Jun 19, 2023
5157711
Simplify reference creation
SupunS Jun 19, 2023
dadf1c3
Simplify reference creation for index expressions
SupunS Jun 19, 2023
b35e6a8
Replace 'pub' with 'access(all)'
SupunS Jun 21, 2023
37c0667
Merge branch 'supun/sync-mutability-restrictions' into supun/member-a…
SupunS Jun 22, 2023
ade5bba
Update test
SupunS Jun 22, 2023
fa638ae
Fix swap statement
SupunS Jun 22, 2023
651f4b5
Merge branch 'feature/mutability-restrictions' of https://github.com/…
SupunS Jun 22, 2023
7cfcc05
Move is-container check to the type itself
SupunS Jun 23, 2023
c16e568
Update tests
SupunS Jun 23, 2023
84e7e0f
Rename MemberInfo to MemberAccessInfo
SupunS Jul 4, 2023
b72a3b1
Update tests
SupunS Jul 4, 2023
26149a0
Fix tidy
SupunS Jul 4, 2023
e47f451
Rename 'MemberAccessible' to 'ContainFieldsOrElements'
SupunS Jul 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion runtime/attachments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,20 @@ func TestAccountAttachmentExportFailure(t *testing.T) {
import Test from 0x1
access(all) fun main(): &Test.A? {
let r <- Test.makeRWithA()
let a = r[Test.A]
var a = r[Test.A]

// 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)
SupunS marked this conversation as resolved.
Show resolved Hide resolved

destroy r
return a
}

access(all) fun returnSameRef(_ ref: &Test.A?): &Test.A? {
return ref
}
`)

runtimeInterface1 := &testRuntimeInterface{
Expand Down
73 changes: 71 additions & 2 deletions runtime/interpreter/interpreter_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, value)
}
},
set: func(value Value) {
Expand All @@ -169,6 +172,12 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a

isNestedResourceMove := interpreter.Program.Elaboration.IsNestedResourceMoveExpression(memberExpression)

memberInfo, ok := interpreter.Program.Elaboration.MemberExpressionMemberInfo(memberExpression)
SupunS marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
panic(errors.NewUnreachableError())
}
memberType := memberInfo.Member.TypeAnnotation.Type

return getterSetter{
target: target,
get: func(allowMissing bool) Value {
Expand Down Expand Up @@ -212,6 +221,13 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a
}
}

// Return a reference, if the member is accessed via a reference.
SupunS marked this conversation as resolved.
Show resolved Hide resolved
// This is pre-computed at the checker.
if memberInfo.ReturnReference {
SupunS marked this conversation as resolved.
Show resolved Hide resolved
// Get a reference to the value
resultValue = interpreter.getReferenceValue(resultValue, memberType)
}

return resultValue
},
set: func(value Value) {
Expand All @@ -222,6 +238,41 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a
}
}

// 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 {
SupunS marked this conversation as resolved.
Show resolved Hide resolved
switch value.(type) {
case NilValue, ReferenceValue:
// Reference to a nil, should return a nil.
// If the value is already a reference then return the same reference.
return value
}

optionalType, ok := semaType.(*sema.OptionalType)
if ok {
semaType = optionalType.Type

// 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)
}

interpreter.maybeTrackReferencedResourceKindedValue(value)
return NewEphemeralReferenceValue(interpreter, UnauthorizedAccess, value, semaType)
}

func (interpreter *Interpreter) checkMemberAccess(
memberExpression *ast.MemberExpression,
target Value,
Expand Down Expand Up @@ -860,10 +911,28 @@ 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, value)
}
}

func (interpreter *Interpreter) maybeGetReference(
expression *ast.IndexExpression,
memberValue Value,
) Value {
indexExpressionTypes := interpreter.Program.Elaboration.IndexExpressionTypes(expression)
if indexExpressionTypes.ReturnReference {
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 {
Expand Down
42 changes: 22 additions & 20 deletions runtime/missingmember_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3542,18 +3542,18 @@ access(all) contract AuctionDutch {
access(all) let currentPrice: UFix64
access(all) let totalItems: Int
access(all) let acceptedBids: Int
access(all) let tickStatus: {UFix64:TickStatus}
access(all) 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
access(all) let tickStatus: &{UFix64:TickStatus}
access(all) let metadata: &{String:String}
turbolent marked this conversation as resolved.
Show resolved Hide resolved

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
}
}

Expand All @@ -3573,7 +3573,7 @@ access(all) contract AuctionDutch {
}

access(all) fun getStatus(_ id: UInt64) : AuctionDutchStatus{
let item= self.getAuction(id)
let item = self.getAuction(id)
let currentTime= 42.0 // Clock.time()

var status="Ongoing"
Expand All @@ -3586,13 +3586,15 @@ access(all) 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,
)
}

access(all) fun getBids(_ id:UInt64) : Bids {
Expand Down
17 changes: 12 additions & 5 deletions runtime/resource_duplicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions runtime/resourcedictionary_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,12 @@ func TestRuntimeResourceDictionaryValues(t *testing.T) {
transaction {

prepare(signer: AuthAccount) {
let c = signer.borrow<&Test.C>(from: /storage/c)!
SupunS marked this conversation as resolved.
Show resolved Hide resolved
let c <- signer.load<@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)
}
}
`)
Expand Down Expand Up @@ -292,10 +293,11 @@ 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)
destroy c.remove("b")
log(c.rs["b"]?.value)
signer.save(<-c, to: /storage/c)
}
}
`)
Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/account_capability_controller.cdc
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
21 changes: 11 additions & 10 deletions runtime/sema/account_capability_controller.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions runtime/sema/anyresource_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
3 changes: 2 additions & 1 deletion runtime/sema/anystruct_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion runtime/sema/authaccount.cdc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

access(all) struct AuthAccount {
access(all) struct AuthAccount: MemberAccessible {

/// The address of the account.
access(all) let address: Address
Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/block.cdc
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

access(all) struct Block {
access(all) struct Block: MemberAccessible {

/// The height of the block.
///
Expand Down
21 changes: 11 additions & 10 deletions runtime/sema/block.gen.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 15 additions & 2 deletions runtime/sema/check_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -316,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,
},
)

Expand Down
Loading