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

fix: builtin const func arg eval #3246

Closed
wants to merge 8 commits into from
Closed

fix: builtin const func arg eval #3246

wants to merge 8 commits into from

Conversation

petar-dambovaliev
Copy link
Contributor

@petar-dambovaliev petar-dambovaliev commented Nov 29, 2024

There are special cases where functions are allowed to be called in a const context.
These functions are

var builtins = []string{"len", "cap", "real", "imag", "complex"}

Currently we only care about len and cap but i added all of them as per spec.
Calling functions in a const context means that the arguments in the call must be constant expressions as well.
This PR implements that.

Go also allows these to be assigned to variables (not only consts), which is handled as well.

Figuring out if its a relevant builtin function by comparing strings is okay because

Builtin functions are not alias-able

  1. They are not assign-able
  2. We do not allow shadowing of builtins
  3. That means there can only be a single way to call the relevant builtin function

Closes #3201

@github-actions github-actions bot added the 📦 🤖 gnovm Issues or PRs gnovm related label Nov 29, 2024
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Nov 29, 2024

Merge Requirements

The following requirements must be fulfilled before a pull request can be merged.
Some requirement checks are automated and can be verified by the CI, while others need manual verification by a staff member.

These requirements are defined in this configuration file.

Automated Checks

🔴 Maintainers must be able to edit this pull request
🟢 The pull request head branch must be up-to-date with its base

Details
Maintainers must be able to edit this pull request

If

🟢 Condition met
└── 🟢 On every pull request

Then

🔴 Requirement not satisfied
└── 🔴 Maintainer can modify this pull request

The pull request head branch must be up-to-date with its base

If

🟢 Condition met
└── 🟢 On every pull request

Then

🟢 Requirement satisfied
└── 🟢 Head branch (3201) is up to date with base (master): behind by 0 / ahead by 8

Manual Checks

No manual checks match this pull request.

Copy link

codecov bot commented Nov 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@Kouteki Kouteki added the in focus Core team is prioritizing this work label Nov 29, 2024
@petar-dambovaliev
Copy link
Contributor Author

I'm closing this as this will be fixed with #2828

@Kouteki Kouteki removed the in focus Core team is prioritizing this work label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🤖 gnovm Issues or PRs gnovm related
Projects
Development

Successfully merging this pull request may close these issues.

Fixed size array failure
3 participants