Bug with inlined functions that access globals
Hi, I think maybe I've seen this brought up once or twice before in the past, but with no real discussion. There is a slightly unpythonic problem with using cpdef inline functions defined in a .pxd file that happen to access global variables. For example: $ cat foo.pxd FOO_A = 1 cpdef inline foo(): return FOO_A $ cat bar.pyx from foo cimport foo print(foo()) Running this like python -c 'import bar' results in: Traceback (most recent call last): File "<string>", line 1, in <module> File "bar.pyx", line 3, in init bar print(foo()) File "foo.pxd", line 4, in foo.foo return FOO_A NameError: name 'FOO_A' is not defined Of course, if the function "foo" were not inlined this would work fine. But the inlining really breaks things, I think, in an unpythonic way. I could be misunderstanding how Cython's "inline" should be interpreted but I feel like it's an implementation detail that should not fundamentally change how the code in that function works. There are a couple issues here. First of all, if a .pxd file exists but not a .pyx, no actual module will be created. This is by design of course, but in this case either, inlined functions in a .pxd module should be prohibited from accessing globals, or a trivial module should be created to store the globals for that module. The second problem boils down to the global variable lookup being compiled to something like: __pyx_t_1 = __Pyx_GetModuleGlobalName(__pyx_n_s_FOO_A) Perhaps, when a function from one module gets inlined in another module, at least in the case where it needs to access globals, the originating module should be imported first and __Pyx_GetModuleGlobalName replaced with an equivalent that uses the originating module's globals. So have a global variable like static PyObject* __pyx_d_foo and in the module init code something like: _pyx_t_1 = __Pyx_Import("foo") __pyx_d_foo = PyModule_GetDict(__pyx_t_1) then in the inlined function, replace the call to __Pyx_GetModuleGlobalName to an equivalent that uses __pyx_d_foo instead of __pyx_d. This all assumes, of course, that "foo" is an importable module. I'm not sure this makes sense otherwise... If not then maybe inlining a function should also "inline" any global variables it uses. Thoughts? Erik
Erik Bray schrieb am 24.11.2017 um 13:44:
I think maybe I've seen this brought up once or twice before in the past, but with no real discussion. There is a slightly unpythonic problem with using cpdef inline functions defined in a .pxd file that happen to access global variables. For example:
$ cat foo.pxd FOO_A = 1
cpdef inline foo(): return FOO_A
Hmm, I might be missing something, but it feels like globals in a .pxd file should not be allowed in the first place - unless there is a .pyx file for them. But that's difficult to prohibit, given that the .pyx file is not needed at compile time, and might not be there at all, just the readily compiled module (and maybe not even that). And I don't see why Python globals should be allowed in .pxd files at all. So - it seems probably difficult to improve the first case. Regarding your actual example, however, if there is really no use case for Python globals in .pxd files, then we should better disallow them. Stefan
On Fri, Nov 24, 2017 at 2:16 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Erik Bray schrieb am 24.11.2017 um 13:44:
I think maybe I've seen this brought up once or twice before in the past, but with no real discussion. There is a slightly unpythonic problem with using cpdef inline functions defined in a .pxd file that happen to access global variables. For example:
$ cat foo.pxd FOO_A = 1
cpdef inline foo(): return FOO_A
Hmm, I might be missing something, but it feels like globals in a .pxd file should not be allowed in the first place - unless there is a .pyx file for them. But that's difficult to prohibit, given that the .pyx file is not needed at compile time, and might not be there at all, just the readily compiled module (and maybe not even that).
And I don't see why Python globals should be allowed in .pxd files at all.
So - it seems probably difficult to improve the first case. Regarding your actual example, however, if there is really no use case for Python globals in .pxd files, then we should better disallow them.
That would be an acceptable resolution, though I believe there is a use case for it. If you take some function that was originally defined in a .pyx module, but you want to be able to inline it, then one is forced to rewrite it in some way if it in any way relied on some module-level globals (which is a normal thing to do). So it's a problem of the implementation details driving what language features one is allowed to use. This is less than ideal... Anyways there's really two issues here: 1) Python globals in .pxd files, which you're saying should be disallowed--I have to wonder if there isn't some way to make that make sense, but I don't feel strongly about it. 2) Non-local variables in functions defined in .pxd files--again these should either be allowed by some mechanism, or entirely disallowed since otherwise the result is either the function breaks with a NameError, or behaves wildly unexpectedly if it gets dropped into a module that happens to have a global of the same name
On Fri, Nov 24, 2017 at 5:48 AM, Erik Bray <erik.m.bray@gmail.com> wrote:
On Fri, Nov 24, 2017 at 2:16 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Erik Bray schrieb am 24.11.2017 um 13:44:
I think maybe I've seen this brought up once or twice before in the past, but with no real discussion. There is a slightly unpythonic problem with using cpdef inline functions defined in a .pxd file that happen to access global variables. For example:
$ cat foo.pxd FOO_A = 1
cpdef inline foo(): return FOO_A
Hmm, I might be missing something, but it feels like globals in a .pxd file should not be allowed in the first place - unless there is a .pyx file for them. But that's difficult to prohibit, given that the .pyx file is not needed at compile time, and might not be there at all, just the readily compiled module (and maybe not even that).
And I don't see why Python globals should be allowed in .pxd files at all.
So - it seems probably difficult to improve the first case. Regarding your actual example, however, if there is really no use case for Python globals in .pxd files, then we should better disallow them.
That would be an acceptable resolution, though I believe there is a use case for it. If you take some function that was originally defined in a .pyx module, but you want to be able to inline it, then one is forced to rewrite it in some way if it in any way relied on some module-level globals (which is a normal thing to do). So it's a problem of the implementation details driving what language features one is allowed to use. This is less than ideal...
Anyways there's really two issues here:
1) Python globals in .pxd files, which you're saying should be disallowed--I have to wonder if there isn't some way to make that make sense, but I don't feel strongly about it. 2) Non-local variables in functions defined in .pxd files--again these should either be allowed by some mechanism, or entirely disallowed since otherwise the result is either the function breaks with a NameError, or behaves wildly unexpectedly if it gets dropped into a module that happens to have a global of the same name
+1 to disallowing python globals altogether in .pxd files. One can cdef any global one needs.
participants (3)
-
Erik Bray -
Robert Bradshaw -
Stefan Behnel