hpy 0.0.4 and c++

[mattip]

this PR

• updates the version of hpy used in tests to 0.0.4
• fixes the code generation to avoid an error in c++ where a goto crosses variable initialization. The fix is to enclose the code in braces

[mattip]

After this is merged, I will create a PR to merge hpy-1 into hpy

[fangerer]

@mattip great, thanks

[mattip]

Hmm. there are some bad failures in the CI I guess we should fix before updating the cython/cython hpy PR

[mattip]

Tests are failing quite quickly. It seems something is wrong with the module level globals. Code like this cython code in refnanny.pyx https://github.com/fangerer/cython/blob/hpy-1/Cython/Runtime/refnanny.pyx#L9-L10 used to emit

 * loglevel = 0             # <<<<<<<<<<<<<<
 * reflog = []
 * 
 */
  if (PyDict_SetItem(__pyx_d, __pyx_n_s_loglevel, __pyx_int_0) < 0) __PYX_ERR(0, 9, __pyx_L1_error)

but now emits

 * loglevel = 0             # <<<<<<<<<<<<<<
 * reflog = []
 * 
 */
  /* __pyx_t_3 allocated (Python object) */
  __pyx_t_3 = __PYX_READ_GLOBAL(__pyx_m, __pyx_n_s_loglevel);
  /* __pyx_t_2 allocated (Python object) */
  __pyx_t_2 = __PYX_READ_GLOBAL(__pyx_m, __pyx_d);
  if (__PYX_API_CALL(__PYX_DICT_SETITEM, __pyx_t_2, __pyx_t_3, __pyx_int_0) < 0) __PYX_ERR(0, 9, __pyx_L1_error)
  __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_3);
  /* __pyx_t_3 released */
  __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_2);
  /* __pyx_t_2 released */

Is there some write-back that needs to happen to get the modified __pyx_t_2 back into the module dictionary __pyx_d ?

[mattip]

The error is that refnanny.loglevel does not exist on the python level.

[mattip]

Using the gcc preprocessor on the code gcc -E -I /usr/include/python3.8 /tmp/refnanny.c -o /tmp/refnanny.c.c shows that the emitted code here is correct, there must be something else going on:

# 7433 "/tmp/refnanny.c"
  __pyx_t_3 = (__pyx_n_s_loglevel);

  __pyx_t_2 = (__pyx_d);
  if (PyDict_SetItem(__pyx_t_2, __pyx_t_3, __pyx_int_0) < 0) { { __pyx_filename = __pyx_f[0]; (void)__pyx_filename; __pyx_lineno = 9; (void)__pyx_lineno; __pyx_clineno = 7436; (void)__pyx_clineno; } goto __pyx_L1_error; }
  ;

 

[mattip]

In the HPy code, the call to __Pyx_InitCachedBuiltins is failing without setting a PyError in the first call to __Pyx_GetBuiltinName("range"). This causes the call to the module init function __pyx_pymod_exec_refnanny to abort before setting up the module attributes.

[mattip]

Spot the error (it took me way too long):

1  static CYTHON_SMALL_CODE int __Pyx_InitCachedBuiltins(__PYX_CONTEXT_DECL0) {
2    __PYX_OBJECT_CTYPE __pyx_t_1 = __PYX_NULL;
3    int __pyx_lineno = 0;
4    const char *__pyx_filename = NULL;
5    int __pyx_clineno = 0;
6    /* __pyx_t_1 allocated (Python object) */
7    __pyx_t_1 = __PYX_API_CALL(__Pyx_GetBuiltinNameFromGlobal, __pyx_n_s_range);
8    if (!__PYX_IS_NULL(__pyx_t_1)) __PYX_ERR(0, 19, __pyx_L1_error)
9    __PYX_WRITE_GLOBAL(__pyx_m, __pyx_builtin_range, __pyx_t_1);
10   __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_1);
 

On line 8 (which in the real code is a single line 7 and 8), the condition is reversed. I think this is a bad interaction with the HPy macro wrapping here

cython/Cython/Compiler/ExprNodes.py

Lines 2340 to 2347 in 4b161ad

	interned_cname = code.intern_identifier(self.entry.name)
	code.globalstate.use_utility_code(
	UtilityCode.load_cached("GetBuiltinName", "ObjectHandling.c"))
	code.putln(
	'%s = %s; %s' % (
	self.result(),
	backend.get_call('__Pyx_GetBuiltinNameFromGlobal', interned_cname),
	code.error_goto_if_null(self.result(), self.pos)))

(EDIT: the problem was somewhere else)

[mattip]

Next up: somehow the wrong variable names are ending up in the code: rather than __pyx_t*, the code generation is printing out the type of the argument (double and list object here):

  __Pyx_GIVEREF(double);
  __PYX_API_CALL(__PYX_TUPLE_BUILDER_SET_ITEM, __pyx_t_4, 0, double);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(list object);
#endif /* CYTHON_COMPILING_IN_HPY */
  __Pyx_GIVEREF(list object);

a more complete code snippet

  /* "_cython_inline_821e59a4f391babb009c56c8e3e7743dbfb68d8b.pyx":6
 * def __invoke(double a, list b):
 * 
 *     return cython.typeof(a), cython.typeof(b)             # <<<<<<<<<<<<<<
 * 
 *     return locals()
 */
  __Pyx_XDECREF(__pyx_r);
  /* __pyx_t_1 allocated (Python object) */
  __pyx_t_1 = __PYX_READ_GLOBAL(__pyx_m, __pyx_n_s_double);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(__pyx_t_1);
#endif /* CYTHON_COMPILING_IN_HPY */
  /* __pyx_t_2 allocated (Python object) */
  __pyx_t_2 = __PYX_READ_GLOBAL(__pyx_m, __pyx_kp_s_list_object);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(__pyx_t_2);
#endif /* CYTHON_COMPILING_IN_HPY */
  /* __pyx_t_3 allocated (Python object) */
  /* __pyx_t_4 allocated (tuple builder) */
  __pyx_t_4 = __PYX_API_CALL(__PYX_TUPLE_BUILDER_NEW, 2);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(double);
#endif /* CYTHON_COMPILING_IN_HPY */
  __Pyx_GIVEREF(double);
  __PYX_API_CALL(__PYX_TUPLE_BUILDER_SET_ITEM, __pyx_t_4, 0, double);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(list object);
#endif /* CYTHON_COMPILING_IN_HPY */
  __Pyx_GIVEREF(list object);
  __PYX_API_CALL(__PYX_TUPLE_BUILDER_SET_ITEM, __pyx_t_4, 1, list object);
  __pyx_t_3 = __PYX_API_CALL(__PYX_TUPLE_BUILDER_BUILD, __pyx_t_4); if (unlikely(__PYX_IS_NULL(__pyx_t_3))) __PYX_ERR(0, 6, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_3);
  /* __pyx_t_4 released */
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = __PYX_NULL;
  __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = __PYX_NULL;
  /* __pyx_t_1 released */
  /* __pyx_t_2 released */
  __pyx_r = __Pyx_NEWREF(__pyx_t_3);
  __pyx_t_3 = __PYX_NULL;
  /* __pyx_t_3 released */
  goto __pyx_L0;

  /* "_cython_inline_821e59a4f391babb009c56c8e3e7743dbfb68d8b.pyx":4
 * cimport cython
 * 
 * def __invoke(double a, list b):             # <<<<<<<<<<<<<<
 * 
 *     return cython.typeof(a), cython.typeof(b)
 */

  /* function exit code */
  __pyx_L1_error:;
  __Pyx_XDECREF(__pyx_t_1);
  __Pyx_XDECREF(__pyx_t_2);
  __Pyx_XDECREF(__pyx_t_3);
  __Pyx_AddTraceback("_cython_inline_821e59a4f391babb009c56c8e3e7743dbfb68d8b.__invoke", __pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_r = __PYX_NULL;
  __pyx_L0:;
  __Pyx_XGIVEREF(__pyx_r);
  __Pyx_RefNannyFinishContext();
  return __pyx_r;
}
#if !CYTHON_COMPILING_IN_HPY
#endif /* !CYTHON_COMPILING_IN_HPY */ 

[mattip]

Also, the use of default character kwarg values is not generating correct code. The CArgDeclNode for a method in a cdef class

1. __PYX_OBJECT_CTYPE values[5] = {__PYX_NULL,__PYX_NULL,__PYX_NULL,__PYX_NULL,__PYX_NULL};
2.
3.    /* "View.MemoryView":127
4. * 
5. *     def __cinit__(array self, tuple shape, Py_ssize_t itemsize, format not None,
6. *                   mode="c", bint allocate_buffer=True):             # <<<<<<<<<<<<<<
7. * 
8. *         cdef int idx
9. */
10.    __pyx_t_1 = __PYX_READ_GLOBAL(__pyx_m, __pyx_n_s_c);
11.   #if !CYTHON_COMPILING_IN_HPY
12. __Pyx_INCREF(__pyx_t_1);
13. #endif /* CYTHON_COMPILING_IN_HPY */
14.     values[3] = ((__PYX_OBJECT_CTYPE)__pyx_t_1);

The value on line 10 __pyx_n_s_c is not created in the interned strings code, and the value __pyx_t_1 is never decrefed, leading to a failure in the cython validate_exit code which notices the leak when generating the code.

Edit: this comes from running the extension_type_memoryview test.
Edit(2): __pyx_n_s_c is indeed properly created, but the failure in validate_exit happens before that code is prepended to code.buffer.

[mattip]

I wonder if there is a way around the many global lookups in the new HPy code. For instance, def foo is turned from

  /* __pyx_t_1 allocated (Python object) */
  __pyx_t_1 = __Pyx_CyFunction_New(&__pyx_mdef_55_cython_inline_dab2a345d7c5cd1da487deb553c203e27f3c76e9_8__invoke_1foo, 0, __pyx_n_s_invoke_locals_foo, NULL, __pyx_n_s_cython_inline_dab2a345d7c5cd1da, __pyx_d, ((PyObject *)__pyx_codeobj__2)); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 5, __pyx_L1_error)
  __Pyx_GOTREF(__pyx_t_1);
  __pyx_v_foo = __pyx_t_1;
  __pyx_t_1 = 0;
  /* __pyx_t_1 released */

into

  #if !CYTHON_COMPILING_IN_HPY
  /* __pyx_t_1 allocated (Python object) */
  __pyx_t_1 = __PYX_READ_GLOBAL(__pyx_m, __pyx_codeobj_);
  #if !CYTHON_COMPILING_IN_HPY
__Pyx_INCREF(__pyx_t_1);
#endif /* CYTHON_COMPILING_IN_HPY */
  /* __pyx_t_2 allocated (Python object) */
  /* __pyx_t_3 allocated (Python object) */
  __pyx_t_3 = __PYX_READ_GLOBAL(__pyx_m, __pyx_n_s_invoke_locals_foo);
  /* __pyx_t_4 allocated (Python object) */
  __pyx_t_4 = __PYX_READ_GLOBAL(__pyx_m, __pyx_n_s_cython_inline_86247832b9dee553f);
  /* __pyx_t_5 allocated (Python object) */
  __pyx_t_5 = __PYX_READ_GLOBAL(__pyx_m, __pyx_d);
  __pyx_t_2 = __PYX_API_CALL(__Pyx_CyFunction_New, &__pyx_mdef_55_cython_inline_86247832b9dee553fd2f4c45122ac751ebaf6951_8__invoke_1foo, 0, __pyx_t_3, NULL, __pyx_t_4, __pyx_t_5, __pyx_t_1); if (unlikely(__PYX_IS_NULL(__pyx_t_2))) __PYX_ERR(0, 5, __pyx_L1_error)
  __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_3);
  /* __pyx_t_3 released */
  __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_4);
  /* __pyx_t_4 released */
  __PYX_CLOSE_LOADED_GLOBAL(__pyx_t_5);
  /* __pyx_t_5 released */
  __Pyx_GOTREF(__pyx_t_2);
  __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = __PYX_NULL;
  /* __pyx_t_1 released */
  __pyx_v_foo = __pyx_t_2;
  __pyx_t_2 = __PYX_NULL;
  /* __pyx_t_2 released */
  #endif /* !CYTHON_COMPILING_IN_HPY */