search

hpyproject / hpy

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

HPyGlobal: debug mode should check registration in HPyModuleDef->globals #462

Open mattip opened 7 months ago

mattip commented 7 months ago

I don't see tests for setting the globals field in HPyModuleDef, and the only reference I could find in the cpython backend is here for error checking.

The NumPy port does use this, but I think that is redundant, since there the objects are globally saved and retrieved but not from the module. Using .global would require being able to fish out the module handles, which is discussed elsewhere.

fangerer commented 7 months ago

You are right. The idea of HPyModuleDef.globals is that one needs to register all HPyGlobal variables such that the interpreter can initialize the C-level value of those and do any internal preparation to be able to use the globals.

For CPython, we don't actually need that because HPyGlobal variables are just C global variables of type PyObject * and those are guaranteed to be initialized to NULL.

But I agree with you: since the contract is that one must register the globals, we should enforce this in the debug mode. We could do that by writing some marker value to the HPyGlobal.

steve-s commented 7 months ago

I though I already had an issue opened for that. I took the liberty and renamed this issue. I think we are using that field in GraalPy (or at least planned to use it) to preallocate our internal array with the actual values for globals.

fangerer commented 7 months ago

yes, we already do that: https://github.com/oracle/graalpython/blob/7279f9c3ee0b9ea6a2bef578af88f575f9ed9f31/graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/cext/hpy/GraalHPyNodes.java#L490

mattip commented 7 months ago

A test would be nice (and that test would fail on CPython since it does not register them).