Introduce `InclusiveRange<T: Integer>` type #2523

darkdrag00nv2 · 2023-06-01T18:52:26Z

Work towards #2482
Work towards onflow/flips#96
Work towards onflow/developer-grants#179

Description

Adds a new InclusiveRange<T: Integer> type to Cadence.

Implemented the following:

Constructor to create the Range
Member variables and functions for the type
Conversion to CompositeValue for easy support for transfer, encoding and decoding

Support for looping inside for-in will be added in a separate PR.

Targeted PR against master branch
Linked to Github issue with discussion and accepted design OR link to spec that describes this work
Code follows the standards mentioned here
Updated relevant documentation
Re-reviewed Files changed in the Github PR explorer
Added appropriate labels

codecov · 2023-06-17T17:20:28Z

Codecov Report

Attention: 136 lines in your changes are missing coverage. Please review.

Comparison is base (edf547b) 79.38% compared to head (4445c56) 79.62%.

❗ Current head 4445c56 differs from pull request most recent head 1ca3adc. Consider uploading reports for the commit 1ca3adc to get more accurate results

Additional details and impacted files

@@                  Coverage Diff                   @@
##           feature/range-type    #2523      +/-   ##
======================================================
+ Coverage               79.38%   79.62%   +0.24%     
======================================================
  Files                     333      336       +3     
  Lines                   78748    79713     +965     
======================================================
+ Hits                    62514    63475     +961     
+ Misses                  13928    13871      -57     
- Partials                 2306     2367      +61

Flag	Coverage Δ
unittests	`79.62% <85.93%> (+0.24%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
encoding/ccf/decode.go	`65.59% <100.00%> (+1.59%)`	⬆️
encoding/ccf/traverse_value.go	`98.33% <100.00%> (ø)`
encoding/json/decode.go	`88.85% <100.00%> (+0.26%)`	⬆️
encoding/json/encode.go	`95.16% <100.00%> (+0.12%)`	⬆️
runtime/common/memorykind_string.go	`40.00% <ø> (ø)`
runtime/common/metering.go	`92.32% <ø> (ø)`
runtime/convertTypes.go	`87.86% <100.00%> (+0.24%)`	⬆️
runtime/sema/errors.go	`93.16% <100.00%> (+0.01%)`	⬆️
runtime/sema/type_tags.go	`94.91% <100.00%> (ø)`
runtime/stdlib/builtin.go	`100.00% <100.00%> (ø)`
... and 17 more

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

runtime/interpreter/value_range.go

turbolent

Great work!

runtime/format/range.go

runtime/interpreter/value_range.go

dsainati1

Can you make a feature/range-type branch that includes all the Range changes, so we can put them in one place them merge them to master together?

darkdrag00nv2 · 2023-06-30T19:11:35Z

@dsainati1 So I don't have the access to do that. Can you please create one for me? I'll target this PR on that branch after that. Thanks!

SupunS · 2023-07-06T14:59:14Z

@darkdrag00nv2 created a new branch feature/range-type and changed the base to it.

runtime/convertTypes.go

runtime/interpreter/decode.go

runtime/interpreter/statictype.go

runtime/interpreter/interpreter.go

runtime/stdlib/range.go

runtime/sema/type_tags.go

runtime/interpreter/value_range.go

runtime/stdlib/range.go

runtime/interpreter/value_range.go

darkdrag00nv2 · 2023-07-21T19:20:29Z

Incorporated half of the suggestions. A few of them are remaining.

runtime/interpreter/interpreter.go

runtime/interpreter/value_range.go

runtime/sema/type.go

darkdrag00nv2 · 2023-07-27T18:01:52Z

@SupunS @dsainati1 This is ready for another round of review. Thanks!

turbolent · 2023-10-03T21:32:41Z

The small integer cache was still not thread-safe and caused data races to be reported (https://github.com/onflow/cadence/actions/runs/6398301138/job/17368137493?pr=2523#step:6:58), so I improved it in 1af52af

turbolent

Coverage is updated now and should be helpful.

runtime/interpreter/value.go

runtime/tests/interpreter/account_test.go

…on test

turbolent

Great work! 👏👏

I think we should get this merged in the current state, and then then potentially remaining work (e.g. making ranges storable) can be addressed in a follow-up PR.

Could you please have a final look @SupunS @dsainati1 ?

runtime/runtime_test.go

SupunS

Should be good to merge in the current state 👍

@darkdrag00nv2 Can you maybe create separate issues for the remaining work / tech-debt, so they won't fall through the cracks

runtime/interpreter/value_range.go

runtime/program_params_validation_test.go

runtime/sema/type.go

darkdrag00nv2 · 2023-10-14T10:38:14Z

@SupunS

Can you maybe create separate issues for the remaining work / tech-debt, so they won't fall through the cracks

Filed #2871 and #2870.

darkdrag00nv2 · 2023-10-18T19:05:05Z

@SupunS Lets merge? I'll send follow-up PRs post that on the fixes and future features such as for-in loop support.

SupunS · 2023-10-18T19:16:40Z

@darkdrag00nv2 Can you please create issues for any remaining open discussions (e.g: #2523 (comment), #2523 (comment)) and resolve those? Then we can merge the PR.

There is a PR restriction that is enforced on this repo, which blocks merging until all conversations are resolved

darkdrag00nv2 · 2023-10-19T16:15:16Z

@SupunS Thanks, Filed all the issues.

This update corresponds to Cadence PR 2523 and subsequent changes to inclusiverange and inclusiverange-type: onflow/cadence#2523 This change does not affect CCF Codec because it was already implemented and deployed in CCF codec.

darkdrag00nv2 mentioned this pull request Jun 3, 2023

Range Types onflow/flips#96

Merged

darkdrag00nv2 force-pushed the range_type branch 2 times, most recently from 03b7e50 to fd72061 Compare June 9, 2023 21:36

darkdrag00nv2 changed the title ~~Introduce Range<T: Integer> type~~ Introduce InclusiveRange<T: Integer> type Jun 19, 2023

darkdrag00nv2 force-pushed the range_type branch 2 times, most recently from dc8d242 to 5b05232 Compare June 25, 2023 19:38

turbolent self-assigned this Jun 28, 2023

turbolent added the Feature label Jun 28, 2023

turbolent assigned SupunS and unassigned turbolent Jun 28, 2023

darkdrag00nv2 marked this pull request as ready for review June 29, 2023 16:50

darkdrag00nv2 requested review from turbolent, SupunS and dsainati1 as code owners June 29, 2023 16:50

darkdrag00nv2 commented Jun 29, 2023

View reviewed changes

runtime/interpreter/value_range.go Outdated Show resolved Hide resolved

darkdrag00nv2 force-pushed the range_type branch from abdda40 to 12eb1cc Compare June 29, 2023 16:58

turbolent reviewed Jun 29, 2023

View reviewed changes

runtime/format/range.go Outdated Show resolved Hide resolved

runtime/interpreter/value_range.go Outdated Show resolved Hide resolved

runtime/interpreter/value_range.go Outdated Show resolved Hide resolved

runtime/interpreter/value_range.go Outdated Show resolved Hide resolved

dsainati1 reviewed Jun 29, 2023

View reviewed changes

darkdrag00nv2 requested review from dsainati1 and turbolent July 3, 2023 19:04

SupunS changed the base branch from master to feature/range-type July 6, 2023 14:58

SupunS reviewed Jul 6, 2023

View reviewed changes

runtime/convertTypes.go Show resolved Hide resolved

runtime/convertTypes.go Show resolved Hide resolved

runtime/interpreter/decode.go Show resolved Hide resolved

runtime/interpreter/statictype.go Show resolved Hide resolved

runtime/interpreter/interpreter.go Outdated Show resolved Hide resolved

SupunS reviewed Jul 7, 2023

View reviewed changes

dsainati1 reviewed Jul 25, 2023

View reviewed changes

runtime/interpreter/interpreter.go Outdated Show resolved Hide resolved

runtime/interpreter/value_range.go Outdated Show resolved Hide resolved

runtime/sema/type.go Outdated Show resolved Hide resolved

darkdrag00nv2 requested a review from dsainati1 July 31, 2023 16:24

turbolent added 3 commits October 3, 2023 13:52

Merge branch 'feature/range-type' into range_type

34a43de

fix qualified string and type ID generation

81aed0f

improve small integer cache

1af52af

remove unnecessary assertion

56c8ac7

turbolent reviewed Oct 3, 2023

View reviewed changes

runtime/interpreter/value.go Show resolved Hide resolved

turbolent reviewed Oct 3, 2023

View reviewed changes

runtime/tests/interpreter/account_test.go Outdated Show resolved Hide resolved

darkdrag00nv2 added 6 commits October 4, 2023 21:49

Separate functions for static type conformance check

9315a88

Add runtime test to save and load InclusiveRange in two transactions

dd77fb7

Revert the TestRuntimeStorageMultipleTransactionsInclusiveRangeFuncti…

d80f426

…on test

Add test case for comparing InclusiveRange

10f48af

Add test case for inclusive range instantiation with 2 type args

64b1b0d

Add test case to indicate unsupported storage of inclusive range

4445c56

darkdrag00nv2 requested a review from turbolent October 6, 2023 16:51

turbolent approved these changes Oct 6, 2023

View reviewed changes

runtime/runtime_test.go Outdated Show resolved Hide resolved

darkdrag00nv2 added 2 commits October 7, 2023 22:01

Make InclusiveRange type non-storable

db22c2c

Remove auth account storage test for InclusiveRange

1ca3adc

dsainati1 approved these changes Oct 10, 2023

View reviewed changes

SupunS approved these changes Oct 13, 2023

View reviewed changes

runtime/interpreter/value_range.go Show resolved Hide resolved

runtime/program_params_validation_test.go Show resolved Hide resolved

runtime/sema/type.go Show resolved Hide resolved

darkdrag00nv2 mentioned this pull request Oct 14, 2023

Improve the smallIntegerValueCache added during InclusiveRange implementation #2871

Closed

SupunS approved these changes Oct 16, 2023

View reviewed changes

SupunS merged commit e523180 into onflow:feature/range-type Oct 19, 2023
7 of 9 checks passed

darkdrag00nv2 deleted the range_type branch October 19, 2023 16:30

SupunS mentioned this pull request Nov 27, 2023

Make checkParameterizedTypeIsInstantiated recursive #2884

Closed

SupunS mentioned this pull request Jan 4, 2024

Add a Range<T: Integer> type and allow in for-loop #2482

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce `InclusiveRange<T: Integer>` type #2523

Introduce `InclusiveRange<T: Integer>` type #2523

darkdrag00nv2 commented Jun 1, 2023 •

edited

Loading

codecov bot commented Jun 17, 2023 •

edited

Loading

turbolent left a comment

dsainati1 left a comment

darkdrag00nv2 commented Jun 30, 2023

SupunS commented Jul 6, 2023

darkdrag00nv2 commented Jul 21, 2023

darkdrag00nv2 commented Jul 27, 2023

turbolent commented Oct 3, 2023

turbolent left a comment

turbolent left a comment

SupunS left a comment

darkdrag00nv2 commented Oct 14, 2023

darkdrag00nv2 commented Oct 18, 2023

SupunS commented Oct 18, 2023 •

edited

Loading

darkdrag00nv2 commented Oct 19, 2023

Introduce InclusiveRange<T: Integer> type #2523

Introduce InclusiveRange<T: Integer> type #2523

Conversation

darkdrag00nv2 commented Jun 1, 2023 • edited Loading

Description

codecov bot commented Jun 17, 2023 • edited Loading

Codecov Report

turbolent left a comment

Choose a reason for hiding this comment

dsainati1 left a comment

Choose a reason for hiding this comment

darkdrag00nv2 commented Jun 30, 2023

SupunS commented Jul 6, 2023

darkdrag00nv2 commented Jul 21, 2023

darkdrag00nv2 commented Jul 27, 2023

turbolent commented Oct 3, 2023

turbolent left a comment

Choose a reason for hiding this comment

turbolent left a comment

Choose a reason for hiding this comment

SupunS left a comment

Choose a reason for hiding this comment

darkdrag00nv2 commented Oct 14, 2023

darkdrag00nv2 commented Oct 18, 2023

SupunS commented Oct 18, 2023 • edited Loading

darkdrag00nv2 commented Oct 19, 2023

Introduce `InclusiveRange<T: Integer>` type #2523

Introduce `InclusiveRange<T: Integer>` type #2523

darkdrag00nv2 commented Jun 1, 2023 •

edited

Loading

codecov bot commented Jun 17, 2023 •

edited

Loading

SupunS commented Oct 18, 2023 •

edited

Loading