Merge pull request #3076 from onflow/bastian/port-v0.42.8-patch.4

runtime/interpreter/errors.go

@@ -340,19 +340,6 @@ func (e DestroyedResourceError) Error() string {
 	return "resource was destroyed and cannot be used anymore"
 }
 
-// ForceAssignmentToNonNilResourceError
-type ForceAssignmentToNonNilResourceError struct {
-	LocationRange
-}
-
-var _ errors.UserError = ForceAssignmentToNonNilResourceError{}
-
-func (ForceAssignmentToNonNilResourceError) IsUserError() {}
-
-func (e ForceAssignmentToNonNilResourceError) Error() string {
-	return "force assignment to non-nil resource-typed value"
-}
-
 // ForceNilError
 type ForceNilError struct {
 	LocationRange
@@ -1008,3 +995,16 @@ func WrappedExternalError(err error) error {
 		return errors.NewExternalError(err)
 	}
 }
+
+// ResourceLossError
+type ResourceLossError struct {
+	LocationRange
+}
+
+var _ errors.UserError = ResourceLossError{}
+
+func (ResourceLossError) IsUserError() {}
+
+func (e ResourceLossError) Error() string {
+	return "resource loss: attempting to assign to non-nil resource-typed value"
+}

runtime/interpreter/interpreter.go

@@ -901,27 +901,15 @@ func (interpreter *Interpreter) visitAssignment(
 		HasPosition: position,
 	}
 
-	// If the assignment is a forced move,
-	// ensure that the target is nil,
-	// otherwise panic
+	// Evaluate the value, and assign it using the setter function
 
-	if transferOperation == ast.TransferOperationMoveForced {
-
-		// If the force-move assignment is used for the initialization of a field,
-		// then there is no prior value for the field, so allow missing
-
-		const allowMissing = true
-
-		target := targetGetterSetter.get(allowMissing)
-
-		if _, ok := target.(NilValue); !ok && target != nil {
-			panic(ForceAssignmentToNonNilResourceError{
-				LocationRange: locationRange,
-			})
-		}
-	}
-
-	// Finally, evaluate the value, and assign it using the setter function
+	// Here it is too early to check whether the existing value is a
+	// valid non-nil resource (i.e: causing a resource loss), because
+	// evaluating the `valueExpression` could change things, and
+	// a `nil`/invalid resource at this point could be valid after
+	// the evaluation of `valueExpression`.
+	// Therefore, delay the checking of resource loss as much as possible,
+	// and check it at the 'setter', at the point where the value is assigned.
 
 	value := interpreter.evalExpression(valueExpression)
 
@@ -5500,3 +5488,31 @@ func (interpreter *Interpreter) withResourceDestruction(
 
 	f()
 }
+
+func (interpreter *Interpreter) checkResourceLoss(value Value, locationRange LocationRange) {
+	if !value.IsResourceKinded(interpreter) {
+		return
+	}
+
+	var resourceKindedValue ResourceKindedValue
+
+	switch existingValue := value.(type) {
+	case *CompositeValue:
+		// A dedicated error is thrown when setting duplicate attachments.
+		// So don't throw an error here.
+		if existingValue.Kind == common.CompositeKindAttachment {
+			return
+		}
+		resourceKindedValue = existingValue
+	case ResourceKindedValue:
+		resourceKindedValue = existingValue
+	default:
+		panic(errors.NewUnreachableError())
+	}
+
+	if !resourceKindedValue.isInvalidatedResource(interpreter) {
+		panic(ResourceLossError{
+			LocationRange: locationRange,
+		})
+	}
+}

runtime/interpreter/interpreter_expression.go

@@ -33,19 +33,19 @@ import (
 
 // assignmentGetterSetter returns a getter/setter function pair
 // for the target expression
-func (interpreter *Interpreter) assignmentGetterSetter(expression ast.Expression) getterSetter {
+func (interpreter *Interpreter) assignmentGetterSetter(expression ast.Expression, locationRange LocationRange) getterSetter {
 	switch expression := expression.(type) {
 	case *ast.IdentifierExpression:
-		return interpreter.identifierExpressionGetterSetter(expression)
+		return interpreter.identifierExpressionGetterSetter(expression, locationRange)
 
 	case *ast.IndexExpression:
 		if attachmentType, ok := interpreter.Program.Elaboration.AttachmentAccessTypes(expression); ok {
 			return interpreter.typeIndexExpressionGetterSetter(expression, attachmentType)
 		}
-		return interpreter.valueIndexExpressionGetterSetter(expression)
+		return interpreter.valueIndexExpressionGetterSetter(expression, locationRange)
 
 	case *ast.MemberExpression:
-		return interpreter.memberExpressionGetterSetter(expression)
+		return interpreter.memberExpressionGetterSetter(expression, locationRange)
 
 	default:
 		return getterSetter{
@@ -61,7 +61,10 @@ func (interpreter *Interpreter) assignmentGetterSetter(expression ast.Expression
 
 // identifierExpressionGetterSetter returns a getter/setter function pair
 // for the target identifier expression
-func (interpreter *Interpreter) identifierExpressionGetterSetter(identifierExpression *ast.IdentifierExpression) getterSetter {
+func (interpreter *Interpreter) identifierExpressionGetterSetter(
+	identifierExpression *ast.IdentifierExpression,
+	locationRange LocationRange,
+) getterSetter {
 	identifier := identifierExpression.Identifier.Identifier
 	variable := interpreter.FindVariable(identifier)
 
@@ -73,6 +76,10 @@ func (interpreter *Interpreter) identifierExpressionGetterSetter(identifierExpre
 		},
 		set: func(value Value) {
 			interpreter.startResourceTracking(value, variable, identifier, identifierExpression)
+
+			existingValue := variable.GetValue()
+			interpreter.checkResourceLoss(existingValue, locationRange)
+
 			variable.SetValue(value)
 		},
 	}
@@ -106,17 +113,34 @@ func (interpreter *Interpreter) typeIndexExpressionGetterSetter(
 
 // valueIndexExpressionGetterSetter returns a getter/setter function pair
 // for the target index expression
-func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression *ast.IndexExpression) getterSetter {
-	target, ok := interpreter.evalExpression(indexExpression.TargetExpression).(ValueIndexableValue)
+func (interpreter *Interpreter) valueIndexExpressionGetterSetter(
+	indexExpression *ast.IndexExpression,
+	locationRange LocationRange,
+) getterSetter {
+
+	// Use getter/setter functions to evaluate the target expression,
+	// instead of evaluating it directly.
+	//
+	// In a swap statement, the left or right side may be an index expression,
+	// and the indexed type (type of the target expression) may be a resource type.
+	// In that case, the target expression must be considered as a nested resource move expression,
+	// i.e. needs to be temporarily moved out (get)
+	// and back in (set) after the index expression got evaluated.
+	//
+	// This is because the evaluation of the index expression
+	// should not be able to access/move the target resource.
+	//
+	// For example, if a side is `a.b[c()]`, then `a.b` is the target expression.
+	// If `a.b` is a resource, then `c()` should not be able to access/move it.
+
+	targetExpression := indexExpression.TargetExpression
+	targetGetterSetter := interpreter.assignmentGetterSetter(targetExpression, locationRange)
+	const allowMissing = false
+	target, ok := targetGetterSetter.get(allowMissing).(ValueIndexableValue)
 	if !ok {
 		panic(errors.NewUnreachableError())
 	}
 
-	locationRange := LocationRange{
-		Location:    interpreter.Location,
-		HasPosition: indexExpression,
-	}
-
 	// Evaluate, transfer, and convert the indexing value,
 	// as it is essentially an "argument" of the get/set operation
 
@@ -136,6 +160,11 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression
 		},
 	)
 
+	isTargetNestedResourceMove := elaboration.IsNestedResourceMoveExpression(targetExpression)
+	if isTargetNestedResourceMove {
+		targetGetterSetter.set(target)
+	}
+
 	// Normally, moves of nested resources (e.g `let r <- rs[0]`) are statically rejected.
 	//
 	// However, there are cases in which we do allow moves of nested resources:
@@ -186,13 +215,13 @@ func (interpreter *Interpreter) valueIndexExpressionGetterSetter(indexExpression
 
 // memberExpressionGetterSetter returns a getter/setter function pair
 // for the target member expression
-func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *ast.MemberExpression) getterSetter {
+func (interpreter *Interpreter) memberExpressionGetterSetter(
+	memberExpression *ast.MemberExpression,
+	locationRange LocationRange,
+) getterSetter {
+
 	target := interpreter.evalExpression(memberExpression.Expression)
 	identifier := memberExpression.Identifier.Identifier
-	locationRange := LocationRange{
-		Location:    interpreter.Location,
-		HasPosition: memberExpression,
-	}
 
 	isNestedResourceMove := interpreter.Program.Elaboration.IsNestedResourceMoveExpression(memberExpression)
 
@@ -243,7 +272,6 @@ func (interpreter *Interpreter) memberExpressionGetterSetter(memberExpression *a
 		},
 		set: func(value Value) {
 			interpreter.checkMemberAccess(memberExpression, target, locationRange)
-
 			interpreter.setMember(target, locationRange, identifier, value)
 		},
 	}
@@ -274,15 +302,8 @@ func (interpreter *Interpreter) checkMemberAccess(
 		}
 	}
 
-	// NOTE: accesses of (optional) storage reference values
-	// are already checked in StorageReferenceValue.dereference
-	_, isStorageReference := target.(*StorageReferenceValue)
-	if !isStorageReference {
-		if optional, ok := target.(*SomeValue); ok {
-			_, isStorageReference = optional.value.(*StorageReferenceValue)
-		}
-	}
-	if isStorageReference {
+	if _, ok := target.(*StorageReferenceValue); ok {
+		// NOTE: Storage reference value accesses are already checked in  StorageReferenceValue.dereference
 		return
 	}
 
@@ -866,7 +887,13 @@ func (interpreter *Interpreter) VisitDictionaryExpression(expression *ast.Dictio
 
 func (interpreter *Interpreter) VisitMemberExpression(expression *ast.MemberExpression) Value {
 	const allowMissing = false
-	return interpreter.memberExpressionGetterSetter(expression).get(allowMissing)
+
+	locationRange := LocationRange{
+		Location:    interpreter.Location,
+		HasPosition: expression,
+	}
+
+	return interpreter.memberExpressionGetterSetter(expression, locationRange).get(allowMissing)
 }
 
 func (interpreter *Interpreter) VisitIndexExpression(expression *ast.IndexExpression) Value {
@@ -1111,11 +1138,14 @@ func (interpreter *Interpreter) VisitCastingExpression(expression *ast.CastingEx
 		HasPosition: expression.Expression,
 	}
 
-	expectedType := interpreter.Program.Elaboration.CastingExpressionTypes(expression).TargetType
+	castingExpressionTypes := interpreter.Program.Elaboration.CastingExpressionTypes(expression)
+
+	expectedType := castingExpressionTypes.TargetType
 
 	switch expression.Operation {
 	case ast.OperationFailableCast, ast.OperationForceCast:
 		valueStaticType := value.StaticType(interpreter)
+		valueSemaType := interpreter.MustConvertStaticToSemaType(valueStaticType)
 		isSubType := interpreter.IsSubTypeOfSemaType(valueStaticType, expectedType)
 
 		switch expression.Operation {
@@ -1124,17 +1154,8 @@ func (interpreter *Interpreter) VisitCastingExpression(expression *ast.CastingEx
 				return Nil
 			}
 
-			// The failable cast may upcast to an optional type, e.g. `1 as? Int?`, so box
-			value = interpreter.BoxOptional(locationRange, value, expectedType)
-
-			// Failable casting is a resource invalidation
-			interpreter.invalidateResource(value)
-
-			return NewSomeValueNonCopying(interpreter, value)
-
 		case ast.OperationForceCast:
 			if !isSubType {
-				valueSemaType := interpreter.MustConvertStaticToSemaType(valueStaticType)
 
 				locationRange := LocationRange{
 					Location:    interpreter.Location,
@@ -1148,15 +1169,24 @@ func (interpreter *Interpreter) VisitCastingExpression(expression *ast.CastingEx
 				})
 			}
 
-			// The failable cast may upcast to an optional type, e.g. `1 as? Int?`, so box
-			return interpreter.BoxOptional(locationRange, value, expectedType)
-
 		default:
 			panic(errors.NewUnreachableError())
 		}
 
+		// The failable cast may upcast to an optional type, e.g. `1 as? Int?`, so box
+		value = interpreter.ConvertAndBox(locationRange, value, valueSemaType, expectedType)
+
+		if expression.Operation == ast.OperationFailableCast {
+			// Failable casting is a resource invalidation
+			interpreter.invalidateResource(value)
+
+			value = NewSomeValueNonCopying(interpreter, value)
+		}
+
+		return value
+
 	case ast.OperationCast:
-		staticValueType := interpreter.Program.Elaboration.CastingExpressionTypes(expression).StaticValueType
+		staticValueType := castingExpressionTypes.StaticValueType
 		// The cast may upcast to an optional type, e.g. `1 as Int?`, so box
 		return interpreter.ConvertAndBox(locationRange, value, staticValueType, expectedType)