Skip to content

Conversation

@BadBastion
Copy link

@BadBastion BadBastion commented Apr 6, 2024

Draft for conversation, this is currently demonstration of what isn't a safe change.

ExecutionEngine Changes

ExecutionEngine<'ctx> -> ExecutionEngine

ExecutionEngine can have more than one module, and thus I believe more than one context. The functions that will break if context is dropped are all related to function execution. In principle this means we should try to ensure safety via JitFunction and not the lifetime of the ExecutionEngine.

JitFunction Changes

JitFunction<'ctx, F> -> JitFunction<F>

The JitFunction<'ctx, F> type is quite a challenging interface for inkwell due to the limited ways to fulfill this lifetime with safe rust. The idiomatic rust would be to change to JitFunction<F> and an Arc to both Execution Engine & Context.

This seems like the most elegant solution from the perspective of thread safe JIT functions, but incurs cascading breaking changes to the rest of the repo.

@BadBastion BadBastion marked this pull request as draft April 6, 2024 19:05
@BadBastion
Copy link
Author

Relevant to #242 and #28

@TheDan64 TheDan64 force-pushed the master branch 3 times, most recently from b7ef5f6 to 7684346 Compare May 18, 2024 05:00
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