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

Add support for resource-kinded fields in transaction roles #2262

Merged
Show file tree
Hide file tree
Changes from 10 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/ast/transaction_declaration.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,10 @@ func (d *TransactionDeclaration) String() string {
// TransactionRoleDeclaration

type TransactionRoleDeclaration struct {
Identifier Identifier
Prepare *SpecialFunctionDeclaration
DocString string
Fields []*FieldDeclaration
Identifier Identifier
Range
}

Expand Down
4 changes: 2 additions & 2 deletions runtime/interpreter/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,10 @@ func (e RedeclarationError) Error() string {
// DereferenceError

type DereferenceError struct {
Cause string
LocationRange
ExpectedType sema.Type
ActualType sema.Type
LocationRange
Cause string
}

var _ errors.UserError = DereferenceError{}
Expand Down
27 changes: 16 additions & 11 deletions runtime/sema/check_composite_declaration.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,17 +171,22 @@ func (checker *Checker) visitCompositeDeclaration(declaration *ast.CompositeDecl

// NOTE: check destructors after initializer and functions

checker.withSelfResourceInvalidationAllowed(func() {
checker.checkDestructors(
declaration.Members.Destructors(),
declaration.Members.FieldsByIdentifier(),
compositeType.Members,
compositeType,
declaration.DeclarationKind(),
declaration.DeclarationDocString(),
kind,
)
})
checker.withResourceFieldInvalidationAllowed(
func(expression *ast.MemberExpression) *Member {
return checker.accessedSelfMember(expression)
},
func() {
checker.checkDestructors(
declaration.Members.Destructors(),
declaration.Members.FieldsByIdentifier(),
compositeType.Members,
compositeType,
declaration.DeclarationKind(),
declaration.DeclarationDocString(),
kind,
)
},
)

// NOTE: visit interfaces first
// DON'T use `nestedDeclarations`, because of non-deterministic order
Expand Down
34 changes: 18 additions & 16 deletions runtime/sema/check_member_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,24 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
IsOptional: isOptional,
},
)

// If the access is to a member which may be invalidated,
// its use must be recorded/checked, so that it isn't used after it was invalidated

resourceFieldInvalidationAllowed := checker.resourceFieldInvalidationAllowed
if resourceFieldInvalidationAllowed != nil {
member := resourceFieldInvalidationAllowed(expression)
if member != nil && member.TypeAnnotation.Type.IsResourceType() {

// NOTE: Preventing the capturing of the resource field is already implicitly handled:
// By definition, the resource field can only be nested in a resource,
// so `self` is a resource, and the capture of it is checked separately

res := Resource{Member: member}

checker.checkResourceUseAfterInvalidation(res, expression.Identifier)
}
}
}()

accessedExpression := expression.Expression
Expand All @@ -133,22 +151,6 @@ func (checker *Checker) visitMember(expression *ast.MemberExpression) (accessedT
return accessedType, member, isOptional
}

// If the access is to a member of `self` and a resource,
// its use must be recorded/checked, so that it isn't used after it was invalidated

accessedSelfMember := checker.accessedSelfMember(expression)
if accessedSelfMember != nil &&
accessedSelfMember.TypeAnnotation.Type.IsResourceType() {

// NOTE: Preventing the capturing of the resource field is already implicitly handled:
// By definition, the resource field can only be nested in a resource,
// so `self` is a resource, and the capture of it is checked separately

res := Resource{Member: accessedSelfMember}

checker.checkResourceUseAfterInvalidation(res, expression.Identifier)
}

identifier := expression.Identifier.Identifier
identifierStartPosition := expression.Identifier.StartPosition()
identifierEndPosition := expression.Identifier.EndPosition(checker.memoryGauge)
Expand Down
25 changes: 21 additions & 4 deletions runtime/sema/check_transaction_declaration.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,30 @@ func (checker *Checker) VisitTransactionDeclaration(declaration *ast.Transaction
declaration.PostConditions,
VoidType,
func() {
checker.withSelfResourceInvalidationAllowed(func() {
checker.visitTransactionExecuteFunction(declaration.Execute, transactionType)
})
checker.withResourceFieldInvalidationAllowed(
func(expression *ast.MemberExpression) *Member {
// In addition to the behaviour in composite destructors,
// where a self member may be invalidated,
// also allow invalidation of role members
memberInfo, _ := checker.Elaboration.MemberExpressionMemberInfo(expression)
if _, ok := memberInfo.AccessedType.(*TransactionRoleType); ok {
return memberInfo.Member
}

return checker.accessedSelfMember(expression)
Copy link
Member Author

@turbolent turbolent Jan 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

},
func() {
checker.visitTransactionExecuteFunction(declaration.Execute, transactionType)
},
)
},
)

checker.checkResourceFieldsInvalidated(transactionType, transactionType.Members)
checker.checkResourceFieldsInvalidated(transactionType, members)

transactionType.Roles.Foreach(func(_ string, roleTye *TransactionRoleType) {
checker.checkResourceFieldsInvalidated(roleTye, roleTye.Members)
})

return
}
Expand Down
149 changes: 74 additions & 75 deletions runtime/sema/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,28 +91,28 @@ type ContractValueHandlerFunc func(

type Checker struct {
// memoryGauge is used for metering memory usage
memoryGauge common.MemoryGauge
Location common.Location
expectedType Type
resources *Resources
valueActivations *VariableActivations
currentMemberExpression *ast.MemberExpression
typeActivations *VariableActivations
containerTypes map[Type]struct{}
Program *ast.Program
PositionInfo *PositionInfo
Config *Config
Elaboration *Elaboration
memoryGauge common.MemoryGauge
Location common.Location
expectedType Type
Config *Config
// initialized lazily. use beforeExtractor()
_beforeExtractor *BeforeExtractor
errors []error
functionActivations *FunctionActivations
inCondition bool
allowSelfResourceFieldInvalidation bool
inAssignment bool
inInvocation bool
inCreate bool
isChecked bool
_beforeExtractor *BeforeExtractor
currentMemberExpression *ast.MemberExpression
typeActivations *VariableActivations
containerTypes map[Type]struct{}
Program *ast.Program
PositionInfo *PositionInfo
resources *Resources
Elaboration *Elaboration
valueActivations *VariableActivations
resourceFieldInvalidationAllowed func(*ast.MemberExpression) *Member
functionActivations *FunctionActivations
errors []error
inCondition bool
inAssignment bool
inInvocation bool
inCreate bool
isChecked bool
}

var _ ast.DeclarationVisitor[struct{}] = &Checker{}
Expand Down Expand Up @@ -1355,9 +1355,10 @@ func (checker *Checker) recordResourceInvalidation(
return nil
}

reportInvalidNestedMove := func() {
reportInvalidNestedMove := func(identifier *ast.Identifier) {
checker.report(
&InvalidNestedResourceMoveError{
Identifier: identifier,
Range: ast.NewRangeFromPositioned(
checker.memoryGauge,
expression,
Expand All @@ -1366,32 +1367,13 @@ func (checker *Checker) recordResourceInvalidation(
)
}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactor of this function is probably best viewed in split-mode. Also, I first refactored the function a bit to simplify it in 7abf054 and to allow customization of the check what is allowed to a function in 48247ab.

accessedSelfMember := checker.accessedSelfMember(expression)

switch expression.(type) {
case *ast.MemberExpression:

if accessedSelfMember == nil ||
!checker.allowSelfResourceFieldInvalidation {

reportInvalidNestedMove()
return nil
getRecordedResourceInvalidation := func(res Resource) *recordedResourceInvalidation {
invalidation := ResourceInvalidation{
Kind: invalidationKind,
StartPos: expression.StartPosition(),
EndPos: expression.EndPosition(checker.memoryGauge),
}

case *ast.IndexExpression:
reportInvalidNestedMove()
return nil
}

invalidation := ResourceInvalidation{
Kind: invalidationKind,
StartPos: expression.StartPosition(),
EndPos: expression.EndPosition(checker.memoryGauge),
}

if checker.allowSelfResourceFieldInvalidation && accessedSelfMember != nil {
res := Resource{Member: accessedSelfMember}

checker.maybeAddResourceInvalidation(res, invalidation)

return &recordedResourceInvalidation{
Expand All @@ -1400,37 +1382,51 @@ func (checker *Checker) recordResourceInvalidation(
}
}

identifierExpression, ok := expression.(*ast.IdentifierExpression)
if !ok {
return nil
}
switch expression := expression.(type) {
case *ast.IdentifierExpression:
variable := checker.findAndCheckValueVariable(expression, false)
if variable == nil {
return nil
}

variable := checker.findAndCheckValueVariable(identifierExpression, false)
if variable == nil {
return nil
}
if invalidationKind != ResourceInvalidationKindMoveTemporary &&
variable.DeclarationKind == common.DeclarationKindSelf {

if invalidationKind != ResourceInvalidationKindMoveTemporary &&
variable.DeclarationKind == common.DeclarationKindSelf {
checker.report(
&InvalidSelfInvalidationError{
InvalidationKind: invalidationKind,
Range: ast.NewRangeFromPositioned(
checker.memoryGauge,
expression,
),
},
)
}

checker.report(
&InvalidSelfInvalidationError{
InvalidationKind: invalidationKind,
Range: ast.NewRangeFromPositioned(
checker.memoryGauge,
expression,
),
},
)
}
return getRecordedResourceInvalidation(Resource{
Variable: variable,
})

case *ast.MemberExpression:
resourceFieldInvalidationAllowed := checker.resourceFieldInvalidationAllowed
if resourceFieldInvalidationAllowed != nil {
member := resourceFieldInvalidationAllowed(expression)
if member != nil {
return getRecordedResourceInvalidation(Resource{
Member: member,
})
}
}

res := Resource{Variable: variable}
reportInvalidNestedMove(&expression.Identifier)
return nil

checker.maybeAddResourceInvalidation(res, invalidation)
case *ast.IndexExpression:
reportInvalidNestedMove(nil)
return nil

return &recordedResourceInvalidation{
resource: res,
invalidation: invalidation,
default:
return nil
}
}

Expand Down Expand Up @@ -1798,11 +1794,14 @@ func (checker *Checker) isWriteableAccess(access ast.Access) bool {
}
}

func (checker *Checker) withSelfResourceInvalidationAllowed(f func()) {
allowSelfResourceFieldInvalidation := checker.allowSelfResourceFieldInvalidation
checker.allowSelfResourceFieldInvalidation = true
func (checker *Checker) withResourceFieldInvalidationAllowed(
invalidationAllowed func(*ast.MemberExpression) *Member,
f func(),
) {
oldInvalidationAllowed := checker.resourceFieldInvalidationAllowed
checker.resourceFieldInvalidationAllowed = invalidationAllowed
defer func() {
checker.allowSelfResourceFieldInvalidation = allowSelfResourceFieldInvalidation
checker.resourceFieldInvalidationAllowed = oldInvalidationAllowed
}()

f()
Expand Down
7 changes: 6 additions & 1 deletion runtime/sema/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -1860,6 +1860,7 @@ func (e *MissingResourceAnnotationError) Error() string {
// InvalidNestedResourceMoveError

type InvalidNestedResourceMoveError struct {
Identifier *ast.Identifier
ast.Range
}

Expand All @@ -1871,6 +1872,11 @@ func (*InvalidNestedResourceMoveError) isSemanticError() {}
func (*InvalidNestedResourceMoveError) IsUserError() {}

func (e *InvalidNestedResourceMoveError) Error() string {
identifier := e.Identifier
if identifier != nil {
return fmt.Sprintf("cannot move nested resource-kinded field `%s`", identifier.Identifier)
}

return "cannot move nested resource"
}

Expand Down Expand Up @@ -2903,7 +2909,6 @@ func (e *ReadOnlyTargetAssignmentError) Error() string {
}

// MissingPrepareForFieldError
//
type MissingPrepareForFieldError struct {
FirstFieldName string
FirstFieldPos ast.Position
Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ import (

*/

// A Resource is a variable or a member
// A Resource is a variable OR a member
type Resource struct {
Variable *Variable
Member *Member
Expand Down
2 changes: 1 addition & 1 deletion runtime/sema/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -5649,10 +5649,10 @@ func IsNilType(ty Type) bool {

type TransactionType struct {
Members *StringMemberOrderedMap
Roles *orderedmap.OrderedMap[string, *TransactionRoleType]
Fields []string
PrepareParameters []Parameter
Parameters []Parameter
Roles *orderedmap.OrderedMap[string, *TransactionRoleType]
}

var _ Type = &TransactionType{}
Expand Down
Loading