Skip to content

refactor: move Value::ProcType to std::function#549

Merged
SuperFola merged 3 commits into
ArkScript-lang:devfrom
27justin:feat/std-function
Jun 10, 2025
Merged

refactor: move Value::ProcType to std::function#549
SuperFola merged 3 commits into
ArkScript-lang:devfrom
27justin:feat/std-function

Conversation

@27justin
Copy link
Copy Markdown
Contributor

@27justin 27justin commented Jun 9, 2025

Description

As discussed in #548, this commit refactors Value::ProcType from a plain C-style function pointer Value(*)(std::vector<Value>&, VM *) into a wrapper class holding a std::function<>.

This enables lambda capture for State::loadFunction, improving flexibility.

This PR also adds two unit tests to test the relevant behaviour.

Checklist

  • I have read the Contributor guide
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation if needed
  • I have added tests that prove my fix/feature is working
  • New and existing tests pass locally with my changes

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 9, 2025

CLA assistant check
All committers have signed the CLA.

@27justin 27justin force-pushed the feat/std-function branch from 814a31e to ea2e09d Compare June 9, 2025 21:36
@27justin 27justin force-pushed the feat/std-function branch from ea2e09d to 0cd5e8d Compare June 9, 2025 21:40
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 10, 2025

Coverage Status

coverage: 86.7% (-0.04%) from 86.738%
when pulling 6c142b6 on 27justin:feat/std-function
into aa74c8f on ArkScript-lang:dev.

Copy link
Copy Markdown
Member

@SuperFola SuperFola left a comment

Choose a reason for hiding this comment

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

Good job, thanks for the PR!

Comment thread include/Ark/VM/State.hpp Outdated
Comment thread include/Ark/VM/Value/Procedure.hpp Outdated
Comment thread include/Ark/VM/Value/Procedure.hpp Outdated
Comment thread tests/unittests/Suites/EmbeddingSuite.cpp Outdated
Comment thread tests/unittests/Suites/EmbeddingSuite.cpp Outdated
@SuperFola
Copy link
Copy Markdown
Member

About the static analysis job, it fails because I badly configured it, I'll fix later no need to worry about it not passing

@SuperFola SuperFola merged commit 58293c7 into ArkScript-lang:dev Jun 10, 2025
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants