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

Port #5653 to feature/atree-inlining-cadence-v0.42 branch #5758

Conversation

fxamacker
Copy link
Member

@fxamacker fxamacker commented Apr 23, 2024

Work towards #5634

This PR ports #5653 to feature branch feature/atree-inlining-cadence-v0.42.

@turbolent Some additional changes are needed due to incompatibility in Cadence v0.42 vs v1.0.

For example, in one test, the InsertWithoutTransfer() function isn't available in Cadence v0.42. When replacing it with Insert(), test payload count is increased by 1, but testing unreferenced slabs isn't affected.

// NOTE: InsertWithoutTransfer isn't available in Cadence v0.42.
// Number of payloads is increased by 1 by using Insert().
dict2.Insert(
inter, interpreter.EmptyLocationRange,
interpreter.NewUnmeteredIntValueFromInt64(2),
interpreter.NewArrayValue(
inter,
interpreter.EmptyLocationRange,
arrayStaticType,
testAddress,
interpreter.NewUnmeteredIntValueFromInt64(3),
),
)

@turbolent
Copy link
Member

If it looks good, we'll also want to port commit 7e3d598 that was added to #5653

@turbolent
Copy link
Member

@fxamacker Looks good, thanks for porting it over! There's a small issue with the dictionary insertion in the test case, I'll push up a fix

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Looks good!

@fxamacker fxamacker requested a review from turbolent April 23, 2024 18:58
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice! Thank you for porting this over and making the required changes 👍

@fxamacker fxamacker requested a review from turbolent April 23, 2024 19:58
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Nice!

@fxamacker fxamacker merged commit c7e0f38 into feature/atree-inlining-cadence-v0.42 Apr 23, 2024
55 checks passed
@fxamacker fxamacker deleted the fxamacker/port-5653-to-feature-atree-inlining-cadence-v0.42 branch April 23, 2024 22:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants