-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Change allocation function logic to be negative. #3
Conversation
9c32837
to
78f50e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking quite nice!
I think we still have a ways to go before we're totally confident about correct-ness, but this is certainly a step in the right direction 👍
const known_alloc_with_throw_funcs = ( | ||
"jl_f_ifelse", "ijl_f_ifelse", | ||
"jl_f_typeassert", "ijl_f_typeassert", | ||
"jl_f_is", "ijl_f_is", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, isn't jl_f_is
non-allocating? jl_egal
is
I don't think jl_f_throw
or jl_throw
are technically allocating either
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's also possible for us to call jl_typeassert
directly
"jl_enter_handler", "ijl_enter_handler", | ||
"jl_pop_handler", "ijl_pop_handler", | ||
"jl_f_typeof", "ijl_f_typeof", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jl_array_data_owner
and jl_object_id
should be in this list too
Changes logic to be safer, where only functions that are known not to allocate are allowed.