C++ STL iteration bugs
Hi, I ran into a couple of problems with the new C++ STL integration, just dumping them here for now. Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code: https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f... Failure to compile iteration over pointer to C++ vector, apparently a type inference bug: https://github.com/cython/cython/commit/edd10c3b33afcac8f471ac6d6664e29f2d98... I may not be able to take a serious look at them within the next week but would like to see them fixed for the release, so I'd be happy if someone can manage to work on them. Stefan
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
Failure to compile iteration over pointer to C++ vector, apparently a type inference bug:
https://github.com/cython/cython/commit/edd10c3b33afcac8f471ac6d6664e29f2d98...
I may not be able to take a serious look at them within the next week but would like to see them fixed for the release, so I'd be happy if someone can manage to work on them.
I'll be taking a poke at them. - Robert
On Sat, Aug 11, 2012 at 10:03 PM, Robert Bradshaw <robertwb@gmail.com> wrote:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
Failure to compile iteration over pointer to C++ vector, apparently a type inference bug:
https://github.com/cython/cython/commit/edd10c3b33afcac8f471ac6d6664e29f2d98...
Well, the problem is that you're trying to iterate over a vector[int]*, and iteration over pointers already has a meaning. Given v of type vector[int]*, it would be nice to support both iteration over v[a:b] and v (element-wise over the pointed-to vector), but I don't think this is a bug per se. (There is similar ugliness with operator overloading, e.g. given "cdef CppClass* x" should "x + 1" increments the pointer rather than operator+.) - Robert
Robert Bradshaw, 12.08.2012 07:12:
On Sat, Aug 11, 2012 at 10:03 PM, Robert Bradshaw wrote:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel wrote:
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Failure to compile iteration over pointer to C++ vector, apparently a type inference bug:
https://github.com/cython/cython/commit/edd10c3b33afcac8f471ac6d6664e29f2d98...
Well, the problem is that you're trying to iterate over a vector[int]*, and iteration over pointers already has a meaning.
Ah, sure.
Given v of type vector[int]*, it would be nice to support both iteration over v[a:b] and v (element-wise over the pointed-to vector), but I don't think this is a bug per se.
Right. However, I don't think iterating over a bare (non-char*) pointer will ever get a meaning, so implementing this by (basically) dereferencing the pointer should be ok. On a somewhat related note, it would be nice to support for x in iter(c_ptr, end_value): ... That would mostly be a generalisation of the special "ends with c'\0'" special case of char* iteration, i.e. that would become equivalent to iter(char_ptr, c'\0')
(There is similar ugliness with operator overloading, e.g. given "cdef CppClass* x" should "x + 1" increments the pointer rather than operator+.)
Hmm, right. That's less ugly though, because the pointer increment actually makes sense. Stefan
Stefan Behnel, 12.08.2012 08:58:
On a somewhat related note, it would be nice to support
for x in iter(c_ptr, end_value): ...
That would mostly be a generalisation of the special "ends with c'\0'" special case of char* iteration, i.e. that would become equivalent to
iter(char_ptr, c'\0')
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain. - Robert
On 12 August 2012 08:00, Robert Bradshaw <robertwb@gmail.com> wrote:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain.
Heh, that should have had a comment :) Cython utility codes are inserted at the AST level in ModuleNode (the merge_in method). This is done since there is no actual C code at that point. I think it could generate it and capture it as well, but it's probably more complicated, and this works well with the name mangling schemes and such.
- Robert _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
On Sun, Aug 12, 2012 at 3:26 AM, mark florisson <markflorisson88@gmail.com> wrote:
On 12 August 2012 08:00, Robert Bradshaw <robertwb@gmail.com> wrote:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain.
Heh, that should have had a comment :) Cython utility codes are inserted at the AST level in ModuleNode (the merge_in method). This is done since there is no actual C code at that point. I think it could generate it and capture it as well, but it's probably more complicated, and this works well with the name mangling schemes and such.
I figured there was a good explanation :). For the moment, we could probably work around it by calling the relevant functions earlier in the pipeline, but it would be nice to get rid of this limitation. - Robert
On 13 August 2012 16:42, Robert Bradshaw <robertwb@gmail.com> wrote:
On Sun, Aug 12, 2012 at 3:26 AM, mark florisson <markflorisson88@gmail.com> wrote:
On 12 August 2012 08:00, Robert Bradshaw <robertwb@gmail.com> wrote:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel <stefan_ml@behnel.de> wrote:
Hi,
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain.
Heh, that should have had a comment :) Cython utility codes are inserted at the AST level in ModuleNode (the merge_in method). This is done since there is no actual C code at that point. I think it could generate it and capture it as well, but it's probably more complicated, and this works well with the name mangling schemes and such.
I figured there was a good explanation :). For the moment, we could probably work around it by calling the relevant functions earlier in the pipeline, but it would be nice to get rid of this limitation.
Yeah, it may work to just pass in the code object or the right insertion point to the code generation function of the AST. I inherited the code and never really touched that part, no idea if that will break things.
- Robert _______________________________________________ cython-devel mailing list cython-devel@python.org http://mail.python.org/mailman/listinfo/cython-devel
Robert Bradshaw, 12.08.2012 09:00:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel wrote:
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain.
I debugged through this a bit and it seems to me that the main problem is that create_from_py_utility_code() is called too late. Currently, it's the code generation phase that calls it (Node.py, around line 2945). That's ok as long as it only generates C code, but it's way too late for Cython utility code at that point. IIRC, the reason why we call it so late is that type conversions can change during several pipeline phases, even way after type analysis, and we want to make sure that we only serialise utility code that is really needed. Sadly, that applies to Cython utility code equally well. Not sure what the right fix it here. I mean, we could call it earlier, even during type analysis, but that might let us generate code that is never needed, thus triggering a couple of C compiler warnings. Maybe that's acceptable at this point (better too much code than incomplete code). We could also try to special case Cython utility code at some place, call create_from_py_utility_code() early and prevent all other types of utility code objects from being injecting into the env at that time, before the code generation phase. Something like that. Stefan
Stefan Behnel, 20.08.2012 20:55:
Robert Bradshaw, 12.08.2012 09:00:
On Sat, Aug 11, 2012 at 1:19 PM, Stefan Behnel wrote:
I ran into a couple of problems with the new C++ STL integration, just dumping them here for now.
Invalid C code when using a stack allocated C++ vector inside of a generator, also lacking type conversion utility code:
https://github.com/cython/cython/commit/d0a8e24720c3ed4e723b5a1b204bf375780f...
The problem here is that utility code attached at this stage of the pipeline gets ignored. In particular, we get to https://github.com/cython/cython/blob/a7c689c10eea66f9fe384d7bc324a7aa50975f... which I am at a loss to explain.
I debugged through this a bit and it seems to me that the main problem is that create_from_py_utility_code() is called too late. Currently, it's the code generation phase that calls it (Node.py, around line 2945). That's ok as long as it only generates C code, but it's way too late for Cython utility code at that point.
IIRC, the reason why we call it so late is that type conversions can change during several pipeline phases, even way after type analysis, and we want to make sure that we only serialise utility code that is really needed. Sadly, that applies to Cython utility code equally well.
Not sure what the right fix it here. I mean, we could call it earlier, even during type analysis, but that might let us generate code that is never needed, thus triggering a couple of C compiler warnings. Maybe that's acceptable at this point (better too much code than incomplete code).
It seems to work if I call it in DefNodeWrapper.analyse_declarations(). Given that we do not do any fancy stuff on argument types anyway, e.g. no type inference, I don't think this can really hurt (but a test run is still pending...). Stefan
participants (3)
-
mark florisson -
Robert Bradshaw -
Stefan Behnel