Skip to content

Conversation

@StanFromIreland
Copy link
Member

@StanFromIreland StanFromIreland commented Apr 23, 2025

@StanFromIreland
Copy link
Member Author

Request: @meadori (Though I think the expert may be inactive :-( )

Copy link
Contributor

@skirpichev skirpichev left a comment

Choose a reason for hiding this comment

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

LGTM

I don't like you added argument index to the module state, but alternatives looks more complex.

CC @serhiy-storchaka

@encukou
Copy link
Member

encukou commented May 5, 2025

AFAICS, putting the argument index in module state is not thread-safe.

I think the best way to solve this would switching all packing operations to use a new per-operation state struct, which would include a (borrowed) pointer to the module state.

@serhiy-storchaka
Copy link
Member

This is a wrong approach. A global variable for the argument index should not be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants