Skip to content

Commit

Permalink
Merge pull request #3017 from onflow/bastian/range-improvements
Browse files Browse the repository at this point in the history
  • Loading branch information
turbolent authored Jan 11, 2024
2 parents 9c630d3 + c3f04ec commit 656acc3
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 86 deletions.
14 changes: 7 additions & 7 deletions encoding/ccf/ccf_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13817,7 +13817,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "nil element type in inclusiverange type",
name: "nil element type in InclusiveRange type",
data: []byte{
// language=edn, format=ccf
// 130([145(nil), [10, 20, 5]])
Expand Down Expand Up @@ -13856,7 +13856,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "invalid array head in inclusiverange value",
name: "invalid array head in InclusiveRange value",
data: []byte{
// language=edn, format=ccf
// 130([145(4), [10, 20, 5]])
Expand Down Expand Up @@ -13896,7 +13896,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "incorrect member count (2 instead of 3) in inclusiverange value",
name: "incorrect member count (2 instead of 3) in InclusiveRange value",
data: []byte{
// language=edn, format=ccf
// 130([145(4), [10, 20, 5]])
Expand Down Expand Up @@ -13931,7 +13931,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "invalid start value in inclusiverange value",
name: "invalid start value in InclusiveRange value",
data: []byte{
// language=edn, format=ccf
// 130([145(5), [10, 20, 5]])
Expand Down Expand Up @@ -13964,7 +13964,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "invalid end value in inclusiverange value",
name: "invalid end value in InclusiveRange value",
data: []byte{
// language=edn, format=ccf
// 130([145(5), [10, 20, 5]])
Expand Down Expand Up @@ -13997,7 +13997,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "invalid step value in inclusiverange value",
name: "invalid step value in InclusiveRange value",
data: []byte{
// language=edn, format=ccf
// 130([145(5), [10, 20, 5]])
Expand Down Expand Up @@ -14226,7 +14226,7 @@ func TestDecodeInvalidData(t *testing.T) {
},
},
{
name: "nil element type in inclusiverange type value",
name: "nil element type in InclusiveRange type value",
data: []byte{
// language=edn, format=ccf
// 130([137(41), 194([null])])
Expand Down
2 changes: 1 addition & 1 deletion encoding/ccf/decode_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (d *Decoder) decodeInclusiveRangeType(
}

if elementType == nil {
return nil, errors.New("unexpected nil type as inclusiverange element type")
return nil, errors.New("unexpected nil type as InclusiveRange element type")
}

return cadence.NewMeteredInclusiveRangeType(d.gauge, elementType), nil
Expand Down
31 changes: 21 additions & 10 deletions runtime/convertValues.go
Original file line number Diff line number Diff line change
Expand Up @@ -1378,7 +1378,7 @@ func (i valueImporter) importInclusiveRangeValue(
inter := i.inter
locationRange := i.locationRange

// start, end and step. The order matters.
// start, end, and step. The order matters.
members := make([]interpreter.IntegerValue, 3)

// import members.
Expand All @@ -1390,15 +1390,21 @@ func (i valueImporter) importInclusiveRangeValue(
importedIntegerValue, ok := importedValue.(interpreter.IntegerValue)
if !ok {
return nil, errors.NewDefaultUserError(
"cannot import inclusiverange: start, end and step must be integers",
"cannot import InclusiveRange: start, end and step must be integers",
)
}

members[index] = importedIntegerValue
}

startValue := members[0]
endValue := members[1]
stepValue := members[2]

startType := startValue.StaticType(inter)

if inclusiveRangeType == nil {
memberSemaType, err := inter.ConvertStaticToSemaType(members[0].StaticType(inter))
memberSemaType, err := inter.ConvertStaticToSemaType(startType)
if err != nil {
return nil, err
}
Expand All @@ -1410,7 +1416,10 @@ func (i valueImporter) importInclusiveRangeValue(
)
}

inclusiveRangeStaticType, ok := interpreter.ConvertSemaToStaticType(inter, inclusiveRangeType).(interpreter.InclusiveRangeStaticType)
inclusiveRangeStaticType, ok := interpreter.ConvertSemaToStaticType(
inter,
inclusiveRangeType,
).(interpreter.InclusiveRangeStaticType)
if !ok {
panic(errors.NewUnreachableError())
}
Expand All @@ -1420,19 +1429,21 @@ func (i valueImporter) importInclusiveRangeValue(
// we do it here because the NewInclusiveRangeValueWithStep constructor performs validations
// which involve comparisons between these values and hence they need to be of the same static
// type.
if members[0].StaticType(inter) != members[1].StaticType(inter) ||
members[0].StaticType(inter) != members[2].StaticType(inter) {

if !startType.Equal(endValue.StaticType(inter)) ||
!startType.Equal(stepValue.StaticType(inter)) {

return nil, errors.NewDefaultUserError(
"cannot import inclusiverange: start, end and step must be of the same type",
"cannot import InclusiveRange: start, end and step must be of the same type",
)
}

return interpreter.NewInclusiveRangeValueWithStep(
inter,
locationRange,
members[0],
members[1],
members[2],
startValue,
endValue,
stepValue,
inclusiveRangeStaticType,
inclusiveRangeType,
), nil
Expand Down
10 changes: 5 additions & 5 deletions runtime/convertValues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1510,7 +1510,7 @@ func TestExportInclusiveRangeValue(t *testing.T) {

t.Parallel()

t.Run("with_step", func(t *testing.T) {
t.Run("with step", func(t *testing.T) {

t.Parallel()

Expand All @@ -1532,7 +1532,7 @@ func TestExportInclusiveRangeValue(t *testing.T) {
assert.Equal(t, expected, actual)
})

t.Run("without_step", func(t *testing.T) {
t.Run("without step", func(t *testing.T) {

t.Parallel()

Expand Down Expand Up @@ -1650,7 +1650,7 @@ func TestImportInclusiveRangeValue(t *testing.T) {
require.Contains(
t,
userError.Error(),
"cannot import inclusiverange: start, end and step must be of the same type",
"cannot import InclusiveRange: start, end and step must be of the same type",
)
})

Expand Down Expand Up @@ -1680,7 +1680,7 @@ func TestImportInclusiveRangeValue(t *testing.T) {
require.Contains(
t,
userError.Error(),
"cannot import inclusiverange: start, end and step must be integers",
"cannot import InclusiveRange: start, end and step must be integers",
)
})
}
Expand Down Expand Up @@ -3559,7 +3559,7 @@ func TestRuntimeMalformedArgumentPassing(t *testing.T) {
expectedInvalidEntryPointArgumentErrType: &MalformedValueError{},
},
{
label: "Malformed inclusiverange",
label: "Malformed InclusiveRange",
typeSignature: "InclusiveRange<Int>",
exportedValue: cadence.NewInclusiveRange(
cadence.NewUInt(1),
Expand Down
4 changes: 2 additions & 2 deletions runtime/interpreter/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3422,7 +3422,7 @@ func TestEncodeDecodeTypeValue(t *testing.T) {
)
})

t.Run("inclusiverange, int", func(t *testing.T) {
t.Run("InclusiveRange, Int", func(t *testing.T) {

t.Parallel()

Expand Down Expand Up @@ -3453,7 +3453,7 @@ func TestEncodeDecodeTypeValue(t *testing.T) {
)
})

t.Run("inclusiverange, uint256", func(t *testing.T) {
t.Run("InclusiveRange, UInt256", func(t *testing.T) {

t.Parallel()

Expand Down
18 changes: 2 additions & 16 deletions runtime/sema/check_invocation_expression.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,20 +464,6 @@ func (checker *Checker) checkInvocation(
}
}

// Compute the invocation range, once, if needed
getInvocationRange := func() func() ast.Range {
var invocationRange ast.Range
return func() ast.Range {
if invocationRange == ast.EmptyRange {
invocationRange = ast.NewRangeFromPositioned(
checker.memoryGauge,
invocationExpression,
)
}
return invocationRange
}
}()

// The invokable type might have special checks for the arguments

if functionType.ArgumentExpressionsCheck != nil && argumentCount > 0 {
Expand All @@ -489,7 +475,7 @@ func (checker *Checker) checkInvocation(
functionType.ArgumentExpressionsCheck(
checker,
argumentExpressions,
getInvocationRange(),
invocationExpression,
)
}

Expand All @@ -514,7 +500,7 @@ func (checker *Checker) checkInvocation(
checker.memoryGauge,
typeArguments,
invocationExpression.TypeArguments,
getInvocationRange(),
invocationExpression,
checker.report,
)
}
Expand Down
22 changes: 14 additions & 8 deletions runtime/sema/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -3868,14 +3868,14 @@ func (t *FunctionType) CheckInstantiated(pos ast.HasPosition, memoryGauge common
type ArgumentExpressionsCheck func(
checker *Checker,
argumentExpressions []ast.Expression,
invocationRange ast.Range,
invocationRange ast.HasPosition,
)

type TypeArgumentsCheck func(
memoryGauge common.MemoryGauge,
typeArguments *TypeParameterTypeOrderedMap,
astTypeArguments []*ast.TypeAnnotation,
astInvocationRange ast.Range,
invocationRange ast.HasPosition,
report func(err error),
)

Expand Down Expand Up @@ -4230,7 +4230,7 @@ var AddressConversionFunctionType = &FunctionType{
},
},
ReturnTypeAnnotation: AddressTypeAnnotation,
ArgumentExpressionsCheck: func(checker *Checker, argumentExpressions []ast.Expression, _ ast.Range) {
ArgumentExpressionsCheck: func(checker *Checker, argumentExpressions []ast.Expression, _ ast.HasPosition) {
if len(argumentExpressions) < 1 {
return
}
Expand Down Expand Up @@ -4317,7 +4317,7 @@ func init() {
}

func numberFunctionArgumentExpressionsChecker(targetType Type) ArgumentExpressionsCheck {
return func(checker *Checker, arguments []ast.Expression, invocationRange ast.Range) {
return func(checker *Checker, arguments []ast.Expression, invocationRange ast.HasPosition) {
if len(arguments) < 1 {
return
}
Expand All @@ -4331,8 +4331,11 @@ func numberFunctionArgumentExpressionsChecker(targetType Type) ArgumentExpressio
checker.Elaboration.SetNumberConversionArgumentTypes(
argument,
NumberConversionArgumentTypes{
Type: targetType,
Range: invocationRange,
Type: targetType,
Range: ast.NewRangeFromPositioned(
checker.memoryGauge,
invocationRange,
),
},
)
}
Expand All @@ -4344,8 +4347,11 @@ func numberFunctionArgumentExpressionsChecker(targetType Type) ArgumentExpressio
checker.Elaboration.SetNumberConversionArgumentTypes(
argument,
NumberConversionArgumentTypes{
Type: targetType,
Range: invocationRange,
Type: targetType,
Range: ast.NewRangeFromPositioned(
checker.memoryGauge,
invocationRange,
),
},
)
}
Expand Down
30 changes: 17 additions & 13 deletions runtime/stdlib/range.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ var inclusiveRangeConstructorFunctionType = func() *sema.FunctionType {
memoryGauge common.MemoryGauge,
typeArguments *sema.TypeParameterTypeOrderedMap,
astTypeArguments []*ast.TypeAnnotation,
astInvocationRange ast.Range,
invocationRange ast.HasPosition,
report func(error),
) {
memberType, ok := typeArguments.Get(typeParameter)
Expand All @@ -89,21 +89,25 @@ var inclusiveRangeConstructorFunctionType = func() *sema.FunctionType {
panic(errors.NewUnreachableError())
}

paramAstRange := astInvocationRange
// If type argument was provided, use its range otherwise fallback to invocation range.
if len(astTypeArguments) > 0 {
paramAstRange = ast.NewRangeFromPositioned(memoryGauge, astTypeArguments[0])
}

// memberType must only be a leaf integer type.
for _, ty := range sema.AllNonLeafIntegerTypes {
if memberType == ty {
report(&sema.InvalidTypeArgumentError{
TypeArgumentName: typeParameter.Name,
Range: paramAstRange,
Details: fmt.Sprintf("Creation of InclusiveRange<%s> is disallowed", memberType),
})
if memberType != ty {
continue
}

// If type argument was provided, use its range otherwise fallback to invocation range.
errorRange := invocationRange
if len(astTypeArguments) > 0 {
errorRange = astTypeArguments[0]
}

report(&sema.InvalidTypeArgumentError{
TypeArgumentName: typeParameter.Name,
Range: ast.NewRangeFromPositioned(memoryGauge, errorRange),
Details: fmt.Sprintf("Creation of InclusiveRange<%s> is disallowed", memberType),
})

break
}
},
}
Expand Down
Loading

0 comments on commit 656acc3

Please sign in to comment.