The buffer protocol does not allow us to just call PyBuffer_Release
and assume the buffer will still be there. Most things that implement the
buffer protocol will let us get away with that, but not all. We need
to release it at the end of the SWIG wrapper.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Event Timeline
Thanks for getting back to this. Is there any reasonable way of testing this (e.g. an existing python class with temporary buffer storage), or would we have to implement our own PyBuffer object?
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
493–495 | please delete public, s/class/struct and delete m_ from m_buffer. (| llvm guidelines say we should use struct for classes with public members, and lldb does not (typically) use the m_ prefix in such classes.) | |
500 | Could you also = delete the copy operations to make sure nothing funny happens with those. |
I don't how of any existing class that does that, I think we'd need to implement our own.
I'm not exactly happy about the lack of a test case, but I guess we can let this slide on the account of it requiring implementing a non-standard PyBuffer in c++ and then loading that from a python test suite and passing it to lldb, which would require a completely new kind of a test case.
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
495 | Move this into the initializer list, or maybe even into the default initializer (Py_buffer buffer = {};). |
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
500 | The = delete is unsupported in SWIG 2, only in 3: http://www.swig.org/Doc3.0/CPlusPlus11.html#CPlusPlus11_defaulted_deleted |
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
500 | It shouldn't be strictly necessary. I put it in so if for some reason one of these values gets copied, it would result in a compiler error instead of a crash. |
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
500 | This type could also just be moved into a header so swig doesn't need to parse it. |
lldb/bindings/python/python-typemaps.swig | ||
---|---|---|
500 | ah nice, of course, I can do this. thanks! |
please delete public, s/class/struct and delete m_ from m_buffer. (| llvm guidelines say we should use struct for classes with public members, and lldb does not (typically) use the m_ prefix in such classes.)