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

Sync stable-cadence branch with master #2968

Merged
merged 19 commits into from
Dec 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion runtime/attachments_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ func TestRuntimeAccountAttachmentExportFailure(t *testing.T) {
},
)
require.Error(t, err)
require.ErrorAs(t, err, &interpreter.DestroyedResourceError{})
require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{})
}

func TestRuntimeAccountAttachmentExport(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion runtime/convertValues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5286,7 +5286,7 @@ func TestRuntimeDestroyedResourceReferenceExport(t *testing.T) {
},
)
require.Error(t, err)
require.ErrorAs(t, err, &interpreter.DestroyedResourceError{})
require.ErrorAs(t, err, &interpreter.InvalidatedResourceReferenceError{})
}

func TestRuntimeDeploymentResultValueImportExport(t *testing.T) {
Expand Down
63 changes: 10 additions & 53 deletions runtime/interpreter/interpreter.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ type Storage interface {
CheckHealth() error
}

type ReferencedResourceKindedValues map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{}
type ReferencedResourceKindedValues map[atree.StorageID]map[*EphemeralReferenceValue]struct{}

type Interpreter struct {
Location common.Location
Expand Down Expand Up @@ -4979,41 +4979,6 @@ func (interpreter *Interpreter) checkContainerMutation(
}
}

func (interpreter *Interpreter) checkReferencedResourceNotDestroyed(value Value, locationRange LocationRange) {
resourceKindedValue, ok := value.(ResourceKindedValue)
if !ok || !resourceKindedValue.IsDestroyed() {
return
}

panic(DestroyedResourceError{
LocationRange: locationRange,
})
}

func (interpreter *Interpreter) checkReferencedResourceNotMovedOrDestroyed(
referencedValue Value,
locationRange LocationRange,
) {

// First check if the referencedValue is a resource.
// This is to handle optionals, since optionals does not
// belong to `ReferenceTrackedResourceKindedValue`

resourceKindedValue, ok := referencedValue.(ResourceKindedValue)
if ok && resourceKindedValue.IsDestroyed() {
panic(DestroyedResourceError{
LocationRange: locationRange,
})
}

referenceTrackedResourceKindedValue, ok := referencedValue.(ReferenceTrackedResourceKindedValue)
if ok && referenceTrackedResourceKindedValue.IsStaleResource(interpreter) {
panic(InvalidatedResourceReferenceError{
LocationRange: locationRange,
})
}
}

func (interpreter *Interpreter) RemoveReferencedSlab(storable atree.Storable) {
storageIDStorable, ok := storable.(atree.StorageIDStorable)
if !ok {
Expand Down Expand Up @@ -5173,23 +5138,26 @@ func (interpreter *Interpreter) ValidateAtreeValue(value atree.Value) {
}

func (interpreter *Interpreter) maybeTrackReferencedResourceKindedValue(value Value) {
if value, ok := value.(ReferenceTrackedResourceKindedValue); ok {
interpreter.trackReferencedResourceKindedValue(value.StorageID(), value)
if referenceValue, ok := value.(*EphemeralReferenceValue); ok {
if value, ok := referenceValue.Value.(ReferenceTrackedResourceKindedValue); ok {
interpreter.trackReferencedResourceKindedValue(value.StorageID(), referenceValue)
}
}
}

func (interpreter *Interpreter) trackReferencedResourceKindedValue(
id atree.StorageID,
value ReferenceTrackedResourceKindedValue,
value *EphemeralReferenceValue,
) {
values := interpreter.SharedState.referencedResourceKindedValues[id]
if values == nil {
values = map[ReferenceTrackedResourceKindedValue]struct{}{}
values = map[*EphemeralReferenceValue]struct{}{}
interpreter.SharedState.referencedResourceKindedValues[id] = values
}
values[value] = struct{}{}
}

// TODO: Remove the `destroyed` flag
SupunS marked this conversation as resolved.
Show resolved Hide resolved
func (interpreter *Interpreter) invalidateReferencedResources(value Value, destroyed bool) {
// skip non-resource typed values
if !value.IsResourceKinded(interpreter) {
Expand Down Expand Up @@ -5232,19 +5200,7 @@ func (interpreter *Interpreter) invalidateReferencedResources(value Value, destr
}

for value := range values { //nolint:maprange
switch value := value.(type) {
case *CompositeValue:
value.dictionary = nil
value.isDestroyed = destroyed
case *DictionaryValue:
value.dictionary = nil
value.isDestroyed = destroyed
case *ArrayValue:
value.array = nil
value.isDestroyed = destroyed
default:
panic(errors.NewUnreachableError())
}
value.Value = nil
SupunS marked this conversation as resolved.
Show resolved Hide resolved
}

// The old resource instances are already cleared/invalidated above.
Expand Down Expand Up @@ -5295,6 +5251,7 @@ func (interpreter *Interpreter) checkInvalidatedResourceUse(
identifier string,
hasPosition ast.HasPosition,
) {

if identifier == sema.SelfIdentifier {
return
}
Expand Down
56 changes: 44 additions & 12 deletions runtime/interpreter/interpreter_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,45 @@ func (interpreter *Interpreter) VisitIdentifierExpression(expression *ast.Identi
}

func (interpreter *Interpreter) evalExpression(expression ast.Expression) Value {
return ast.AcceptExpression[Value](expression, interpreter)
result := ast.AcceptExpression[Value](expression, interpreter)
interpreter.checkInvalidatedResourceOrResourceReference(result, expression)
return result
}

func (interpreter *Interpreter) checkInvalidatedResourceOrResourceReference(value Value, hasPosition ast.HasPosition) {
// Unwrap SomeValue, to access references wrapped inside optionals.
someValue, isSomeValue := value.(*SomeValue)
for isSomeValue && someValue.value != nil {
value = someValue.value
someValue, isSomeValue = value.(*SomeValue)
}

switch value := value.(type) {
case ResourceKindedValue:
if value.isInvalidatedResource(interpreter) {
panic(InvalidatedResourceError{
LocationRange: LocationRange{
Location: interpreter.Location,
HasPosition: hasPosition,
},
})
}
case *EphemeralReferenceValue:
if value.Value == nil {
panic(InvalidatedResourceReferenceError{
LocationRange: LocationRange{
Location: interpreter.Location,
HasPosition: hasPosition,
},
})
} else {
// If the value is there, check whether the referenced value is an invalidated one.
// This step is not really needed, since reference tracking is supposed to clear the
// `value.Value` if the referenced-value was moved/deleted.
// However, have this as a second layer of defensive.
interpreter.checkInvalidatedResourceOrResourceReference(value.Value, hasPosition)
}
}
}

func (interpreter *Interpreter) VisitBinaryExpression(expression *ast.BinaryExpression) Value {
Expand Down Expand Up @@ -1036,15 +1074,6 @@ func (interpreter *Interpreter) visitInvocationExpressionWithImplicitArgument(in
panic(errors.NewUnreachableError())
}

// Bound functions
if boundFunction, ok := function.(BoundFunctionValue); ok && boundFunction.Self != nil {
self := *boundFunction.Self
// Explicitly track the reference here, because the receiver 'act' as a reference,
// but a reference is never created during bound function invocation.
// This was a fix to the issue: https://github.com/onflow/cadence/pull/2561
interpreter.maybeTrackReferencedResourceKindedValue(self)
}

// NOTE: evaluate all argument expressions in call-site scope, not in function body

var argumentExpressions []ast.Expression
Expand Down Expand Up @@ -1341,14 +1370,17 @@ func (interpreter *Interpreter) VisitReferenceExpression(referenceExpression *as

case *sema.ReferenceType:
// Case (3): target type is non-optional, actual value is optional.
// Unwrap the optional and add it to reference tracking.
// This path shouldn't be reachable. This is only a defensive step
// to ensure references are properly created/tracked.
if someValue, ok := result.(*SomeValue); ok {
locationRange := LocationRange{
Location: interpreter.Location,
HasPosition: referenceExpression.Expression,
}
innerValue := someValue.InnerValue(interpreter, locationRange)
interpreter.maybeTrackReferencedResourceKindedValue(innerValue)

auth := interpreter.getEffectiveAuthorization(typ)
return NewEphemeralReferenceValue(interpreter, auth, innerValue, typ.Type, locationRange)
}

// Case (4): target type is non-optional, actual value is also non-optional
Expand Down
2 changes: 1 addition & 1 deletion runtime/interpreter/sharedstate.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func NewSharedState(config *Config) *SharedState {
},
inStorageIteration: false,
storageMutatedDuringIteration: false,
referencedResourceKindedValues: map[atree.StorageID]map[ReferenceTrackedResourceKindedValue]struct{}{},
referencedResourceKindedValues: map[atree.StorageID]map[*EphemeralReferenceValue]struct{}{},
resourceVariables: map[ResourceKindedValue]*Variable{},
CapabilityControllerIterations: map[AddressPath]int{},
containerValueIteration: map[atree.StorageID]struct{}{},
Expand Down
Loading
Loading