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

feat(hog): inline STL #24636

Closed
wants to merge 1 commit into from
Closed

feat(hog): inline STL #24636

wants to merge 1 commit into from

Conversation

mariusandra
Copy link
Collaborator

@mariusandra mariusandra commented Aug 28, 2024

Problem

Last part extracted from #24369

I'm not 100% convinced we should do it this way... but this works today and should be backwards compatible in case we want any other changes.

Changes

Adds support for arrayMap, arrayExists, arrayFilter:

print(arrayMap(x -> x * 2, [1,2,3]))
print(arrayExists(x -> x like '%nana%', ['apple', 'banana', 'cherry']))
print(arrayFilter(x -> x like '%e%', ['apple', 'banana', 'cherry']))

This is where it gets a bit tricky. Those are added via an "inlined STL". Effectively the bytecode compiler does a round of static analysis and figures out if any of those STL functions will be called. If so, it appends source code for the function before your script.

All three functions are written as Hog functions behind the scenes, for example:

fn arrayExists(func, arr) {
  for (let i in arr) {
    if (func(i)) {
      return true
    }
  }
  return false
}

This was easier (and safer) to get working than implementing a back-and-forth layer between the VM and "native code". We can today call native code from the VM (all those STL functions), but to the do an UNO reverse and call a VM function from native code... requires a bigger refactor.

Currently those STL functions are inlined into the start of Hog bytecode. I'm not sure if we want to keep it that way, or ship the function bytecodes with each HogVM itself.

There are some positive things about having a STL written within Hog: 1) it's easier to extend, and 2) it requires less changes in all the different implementations of Hog (Python vs TS vs future Rust?)

Alternatives

The current option isn't very clean:

  • It makes all bytecode using e.g. arrayMap longer
  • There's no recursion between "inline STL" functions (e.g. arrayReduce can't depend on arrayMap)
  • You can't have your own top level functions or variables with the name of the STL functions.

The alternatives to this are to either:

  • Have native code call Hog code/lambdas/closures directly. I still want to try if this is not too horrible.
  • Precompile all STL functions into bytecode, ship those with each HogVM and call that bytecode directly instead of inlining during compilation.

Those options sound cleaner/better in some way.

Out of scope

Still out of scope for this PR is getting the elements chain matching working. This requires some extra fields and is a WIP here.

How did you test this code?

Hog snapshot test for inline STL functions.

@mariusandra
Copy link
Collaborator Author

Scrapping this and going with #24643 instead. Feels cleaner.

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.

1 participant