Skip to content

Commit

Permalink
Merge pull request #2725 from onflow/supun/improve-conformance
Browse files Browse the repository at this point in the history 
Allow default functions to co-exist with empty function declarations
  • Loading branch information
@SupunS
SupunS authored Oct 25, 2023
2 parents f6eb669 + ddc8181 commit 01a42f3
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 214 deletions.
127 changes: 0 additions & 127 deletions runtime/error_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,133 +485,6 @@ func TestRuntimeError(t *testing.T) {
})
}

func TestRuntimeDefaultFunctionConflictPrintingError(t *testing.T) {
t.Parallel()

runtime := NewTestInterpreterRuntime()

makeDeployTransaction := func(name, code string) []byte {
return []byte(fmt.Sprintf(
`
transaction {
prepare(signer: auth(BorrowValue) &Account) {
let acct = Account(payer: signer)
acct.contracts.add(name: "%s", code: "%s".decodeHex())
}
}
`,
name,
hex.EncodeToString([]byte(code)),
))
}

contractInterfaceCode := `
access(all) contract TestInterfaces {
access(all) resource interface A {
access(all) fun foo() {
let x = 3
}
}
access(all) resource interface B {
access(all) fun foo()
}
}
`

contractCode := `
import TestInterfaces from 0x2
access(all) contract TestContract {
access(all) resource R: TestInterfaces.A, TestInterfaces.B {}
// fill space
// fill space
// fill space
// fill space
// fill space
// fill space
// filling lots of space
// filling lots of space
// filling lots of space
}
`

accountCodes := map[Location][]byte{}
var events []cadence.Event

var nextAccount byte = 0x2

runtimeInterface := &TestRuntimeInterface{
OnGetCode: func(location Location) (bytes []byte, err error) {
return accountCodes[location], nil
},
Storage: NewTestLedger(nil, nil),
OnCreateAccount: func(payer Address) (address Address, err error) {
result := interpreter.NewUnmeteredAddressValueFromBytes([]byte{nextAccount})
nextAccount++
return result.ToAddress(), nil
},
OnGetSigningAccounts: func() ([]Address, error) {
return []Address{{0x1}}, nil
},
OnResolveLocation: NewSingleIdentifierLocationResolver(t),
OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) {
return accountCodes[location], nil
},
OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error {
accountCodes[location] = code
return nil
},
OnEmitEvent: func(event cadence.Event) error {
events = append(events, event)
return nil
},
}

nextTransactionLocation := NewTransactionLocationGenerator()

deployTransaction := makeDeployTransaction("TestInterfaces", contractInterfaceCode)
err := runtime.ExecuteTransaction(
Script{
Source: deployTransaction,
},
Context{
Interface: runtimeInterface,
Location: nextTransactionLocation(),
},
)
require.NoError(t, err)

deployTransaction = makeDeployTransaction("TestContract", contractCode)
err = runtime.ExecuteTransaction(
Script{
Source: deployTransaction,
},
Context{
Interface: runtimeInterface,
Location: nextTransactionLocation(),
},
)
require.Error(t, err)
require.Contains(t, err.Error(), "access(all) resource R: TestInterfaces.A, TestInterfaces.B {}")

var errType *sema.CheckerError
require.ErrorAs(t, err, &errType)

checkerErr := err.(Error).
Err.(interpreter.Error).
Err.(*stdlib.InvalidContractDeploymentError).
Err.(*ParsingCheckingError).
Err.(*sema.CheckerError)

var specificErrType *sema.DefaultFunctionConflictError
require.ErrorAs(t, checkerErr.Errors[0], &specificErrType)

errorRange := checkerErr.Errors[0].(*sema.DefaultFunctionConflictError).Range

require.Equal(t, errorRange.StartPos.Line, 4)
}

func TestRuntimeMultipleInterfaceDefaultImplementationsError(t *testing.T) {
t.Parallel()

Expand Down
27 changes: 0 additions & 27 deletions runtime/sema/check_composite_declaration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1436,33 +1436,6 @@ func (checker *Checker) checkMemberConflicts(

return true
}

// At most one of them have could default impls.
// If one has a default impl, then the other MUST have a condition.
// FLIP: https://github.com/onflow/flips/pull/83

if newMember.HasImplementation {
if existingMember.HasConditions {
continue
}
} else if existingMember.HasImplementation {
if newMember.HasConditions {
continue
}
} else {
// None of them have default impls
continue
}

checker.report(
&DefaultFunctionConflictError{
CompositeKindedType: compositeType,
Member: newMember,
Range: errorRange,
},
)

return true
}

return false
Expand Down
34 changes: 22 additions & 12 deletions runtime/sema/check_interface_declaration.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ func (checker *Checker) checkInterfaceConformance(

var isDuplicate bool

// Check if the members coming from other conformances have conflicts.
// Check if the members coming from other conformances (siblings) have conflicts.
inheritedMembers, ok := inheritedMembersByName[name]
if ok {
for _, conflictingMember := range inheritedMembers {
Expand All @@ -609,6 +609,7 @@ func (checker *Checker) checkInterfaceConformance(
conflictingInterface,
conflictingMember,
interfaceDeclaration.Identifier,
false, // conflicting member is a sibling
)
}
}
Expand All @@ -622,6 +623,7 @@ func (checker *Checker) checkInterfaceConformance(
conformance,
conformanceMember,
declarationMember.Identifier,
true, // conflicting member is an inherited member
)
}

Expand All @@ -639,6 +641,7 @@ func (checker *Checker) checkDuplicateInterfaceMember(
conflictingInterfaceType *InterfaceType,
conflictingMember *Member,
hasPosition ast.HasPosition,
isConflictingMemberInherited bool,
) (isDuplicate bool) {

reportMemberConflictError := func() {
Expand Down Expand Up @@ -672,24 +675,31 @@ func (checker *Checker) checkDuplicateInterfaceMember(
// Having conflicting identical functions with:
// - (1) and (1) - OK
// - (1) and (2) - OK
// - (1) and (3) - Not OK
// - (1) and (3) - OK
// - (2) and (1) - OK
// - (2) and (2) - OK
// - (2) and (3) - OK
// - (3) and (1) - Not OK (order matters)
// - (3) and (2) - OK
// - (3) and (3) - Not OK

if interfaceMember.HasImplementation {
if conflictingMember.HasImplementation || !conflictingMember.HasConditions {
reportMemberConflictError()
return
}
if interfaceMember.HasImplementation && conflictingMember.HasImplementation {
reportMemberConflictError()
return
}

if conflictingMember.HasImplementation {
if !interfaceMember.HasConditions {
reportMemberConflictError()
return
}
// If the conflicting member is an inherited member, it is OK to override
// the inherited declaration, by a default implementation.
// However, a default implementation cannot be overridden by an empty declaration.

if isConflictingMemberInherited &&
conflictingMember.HasImplementation && !interfaceMember.HasConditions {
reportMemberConflictError()
return
}

// If the conflicting member is not an inherited one, (i.e.a member from a sibling conformance)
// then default implementation takes the precedence.

return
}
23 changes: 0 additions & 23 deletions runtime/sema/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1645,29 +1645,6 @@ func (e *SpecialFunctionDefaultImplementationError) EndPosition(memoryGauge comm
return e.Identifier.EndPosition(memoryGauge)
}

// DefaultFunctionConflictError
type DefaultFunctionConflictError struct {
CompositeKindedType CompositeKindedType
Member *Member
ast.Range
}

var _ SemanticError = &DefaultFunctionConflictError{}
var _ errors.UserError = &DefaultFunctionConflictError{}

func (*DefaultFunctionConflictError) isSemanticError() {}

func (*DefaultFunctionConflictError) IsUserError() {}

func (e *DefaultFunctionConflictError) Error() string {
return fmt.Sprintf(
"%s `%s` has conflicting requirements for function `%s`",
e.CompositeKindedType.GetCompositeKind().Name(),
e.CompositeKindedType.QualifiedString(),
e.Member.Identifier.Identifier,
)
}

// InterfaceMemberConflictError
type InterfaceMemberConflictError struct {
InterfaceType *InterfaceType
Expand Down
50 changes: 25 additions & 25 deletions runtime/tests/checker/interface_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1894,9 +1894,7 @@ func TestCheckMultipleInterfaceSingleInterfaceDefaultImplementation(t *testing.T
}
`)

errs := RequireCheckerErrors(t, err, 1)

require.IsType(t, &sema.DefaultFunctionConflictError{}, errs[0])
require.NoError(t, err)
})
}

Expand Down Expand Up @@ -3017,12 +3015,28 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
}
`)

errs := RequireCheckerErrors(t, err, 1)
require.NoError(t, err)
})

memberConflictError := &sema.InterfaceMemberConflictError{}
require.ErrorAs(t, errs[0], &memberConflictError)
assert.Equal(t, "hello", memberConflictError.MemberName)
assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
t.Run("default impl in child, declaration in parent, concrete type", func(t *testing.T) {

t.Parallel()

_, err := ParseAndCheck(t, `
struct interface A {
access(all) fun hello()
}
struct interface B: A {
access(all) fun hello() {
var a = 1
}
}
struct C: B {}
`)

require.NoError(t, err)
})

t.Run("default impl in both", func(t *testing.T) {
Expand Down Expand Up @@ -3073,7 +3087,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
}
`)

errs := RequireCheckerErrors(t, err, 3)
errs := RequireCheckerErrors(t, err, 2)

memberConflictError := &sema.InterfaceMemberConflictError{}
require.ErrorAs(t, errs[0], &memberConflictError)
Expand All @@ -3084,11 +3098,6 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
require.ErrorAs(t, errs[1], &memberConflictError)
assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
assert.Equal(t, "hello", memberConflictError.MemberName)
assert.Equal(t, "B", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())

require.ErrorAs(t, errs[2], &memberConflictError)
assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
assert.Equal(t, "hello", memberConflictError.MemberName)
assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
})

Expand Down Expand Up @@ -3360,13 +3369,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
struct interface D: A, B, C {}
`)

errs := RequireCheckerErrors(t, err, 1)

memberConflictError := &sema.InterfaceMemberConflictError{}
require.ErrorAs(t, errs[0], &memberConflictError)
assert.Equal(t, "hello", memberConflictError.MemberName)
assert.Equal(t, "A", memberConflictError.ConflictingInterfaceType.QualifiedIdentifier())
assert.Equal(t, "C", memberConflictError.InterfaceType.QualifiedIdentifier())
require.NoError(t, err)
})

t.Run("all three formats of function, concrete type", func(t *testing.T) {
Expand All @@ -3393,10 +3396,7 @@ func TestCheckInterfaceDefaultMethodsInheritance(t *testing.T) {
struct D: A, B, C {}
`)

errs := RequireCheckerErrors(t, err, 1)

defaultFunctionConflictError := &sema.DefaultFunctionConflictError{}
require.ErrorAs(t, errs[0], &defaultFunctionConflictError)
require.NoError(t, err)
})
}

Expand Down

0 comments on commit 01a42f3

Please sign in to comment.