hpyproject / hpy

HPy: a better API for Python
https://hpyproject.org
MIT License
1.02k stars 52 forks source link

Debug mode: check correct HPyContext usage #363

Closed steve-s closed 1 year ago

steve-s commented 2 years ago

Based on WIP in https://github.com/hpyproject/hpy/pull/350 done with @NiRit100 at the HPy sprint

mattip commented 1 year ago

This has a merge conflict. Is it ready for review?

steve-s commented 1 year ago

I've rebased the PR and it's ready for a review now.

steve-s commented 1 year ago

@antocuni would you have some spare cycles for a review :-)?

mattip commented 1 year ago

This has conflicts.

steve-s commented 1 year ago

Proactively rebased on top of https://github.com/hpyproject/hpy/pull/402 to streamline merging of both PRs

mattip commented 1 year ago

Could you describe under what cases the checks might fail? What is the scenario that would cause someone to get the wrong context?

steve-s commented 1 year ago

Could you describe under what cases the checks might fail? What is the scenario that would cause someone to get the wrong context?

What this aims to prevent is a situation when someone misunderstands the contract of HPy and stashes context into some global variable to read it later in a context of another new Python->HPy call. For example, in the test:

https://github.com/hpyproject/hpy/pull/363/files#diff-885e9f36d2a0124e4a50c595a7fac468a1ac9af3c5dea852646c1d52d5694346R37

on this line it uses global variable keep as HPyContext, but it should use the ctx argument.

mattip commented 1 year ago

Thanks @steve-s