[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna marked an inline comment as done. lawrence_danna added inline comments. Comment at: lldb/trunk/scripts/Python/python-typemaps.swig:481 + PyBuffer_Release(&view); + $1 = ($1_ltype) buf; + $2 = ($2_ltype) (size/sizeof($*1_type)); labath wrote: > v

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added inline comments. Comment at: lldb/trunk/scripts/Python/python-typemaps.swig:481 + PyBuffer_Release(&view); + $1 = ($1_ltype) buf; + $2 = ($2_ltype) (size/sizeof($*1_type)); vadimcn wrote: > Sorry for being late to the party, but I just stumbled up

[Lldb-commits] [PATCH] D67793: new api class: SBFile

vadimcn added inline comments. Comment at: lldb/trunk/scripts/Python/python-typemaps.swig:481 + PyBuffer_Release(&view); + $1 = ($1_ltype) buf; + $2 = ($2_ltype) (size/sizeof($*1_type)); Sorry for being late to the party, but I just stumbled upon this code...

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added inline comments. Comment at: lldb/trunk/source/Host/common/File.cpp:74 + .Case("r", File::eOpenOptionRead) + .Case("w", File::eOpenOptionWrite) + .Case("a", File::eOpenOptionWrite | File::eOpenOptionAppend | mgorny wrote: > Shouldn't t

[Lldb-commits] [PATCH] D67793: new api class: SBFile

mgorny added a comment. While looking at the other bug, I noticed something suspicious here. Comment at: lldb/trunk/source/Host/common/File.cpp:74 + .Case("r", File::eOpenOptionRead) + .Case("w", File::eOpenOptionWrite) + .Case("a", File::eOpenOptionWrite | File:

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added a comment. In D67793#1713981 , @lawrence_danna wrote: > In D67793#1692544 , @labath wrote: > > > BTW, I've had to add (or rather, extend) a fairly ugly hack in r373573 in > > order to get the new test

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. In D67793#1692544 , @labath wrote: > BTW, I've had to add (or rather, extend) a fairly ugly hack in r373573 in > order to get the new tests running on non-darwin platforms. The reason for > that is that the FILE* out typem

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added a comment. BTW, I've had to add (or rather, extend) a fairly ugly hack in r373573 in order to get the new tests running on non-darwin platforms. The reason for that is that the FILE* out typemap needs to get the "mode" of the object in order to construct the python wrapper. My hop

[Lldb-commits] [PATCH] D67793: new api class: SBFile

This revision was automatically updated to reflect the committed changes. Closed by commit rL373562: new api class: SBFile (authored by lawrence_danna, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D67793

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Nah, this looks fine to me now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. @labath any more comments on this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 ___ lldb-commits mailing list lldb-commit

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 222661. lawrence_danna added a comment. no inlines in the API Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h lldb/includ

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna marked an inline comment as done. lawrence_danna added inline comments. Comment at: lldb/scripts/Python/python-typemaps.swig:464 } + +// These two pybuffer macros are copied out of swig/Lib/python/pybuffer.i, `swig/LICENSE` says the following, so

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 222528. lawrence_danna added a comment. add reproducer instrumentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h l

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 222522. lawrence_danna added a comment. flush fix for python2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h lldb/includ

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. addressed comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 ___ lldb-commits mailing list lldb-commits@lists.llvm.org htt

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 222516. lawrence_danna marked 7 inline comments as done. lawrence_danna added a comment. fixed according to reviewer comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm

[Lldb-commits] [PATCH] D67793: new api class: SBFile

jingham added a comment. This looks fine to me, it doesn't look like you are leaving out anything useful. Note, I think just to enforce the "default constructor" discipline, there's a test in the test suite (python_api/default-constructor/TestDefaultConstructorForAPIObjects.py) that has a file

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added a comment. Thanks. This looks good to me, but given that this is going to become a stable api that well need to maintain for a looong time, I'd like to get signoff from someone else as well. @jingham maybe ? Comment at: lldb/include/lldb/API/SBDefines.h:95 class

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 64. lawrence_danna added a comment. rebased, removed duplicate test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h l

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna marked 2 inline comments as done. lawrence_danna added a comment. @labath, OK sets are now constructors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 222190. lawrence_danna edited the summary of this revision. lawrence_danna added a comment. rebased, and converted Set* methods into constructors. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added inline comments. Comment at: lldb/source/API/SBFile.cpp:20-26 +void SBFile::SetStream(FILE *file, bool transfer_ownership) { +m_opaque_up = std::make_unique(file, transfer_ownership); +} + +void SBFile::SetDescriptor(int fd, bool transfer_owndership) { +m_opa

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna marked 2 inline comments as done. lawrence_danna added inline comments. Comment at: lldb/source/API/SBFile.cpp:20-26 +void SBFile::SetStream(FILE *file, bool transfer_ownership) { +m_opaque_up = std::make_unique(file, transfer_ownership); +} + +void SBFile::Set

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 221627. lawrence_danna added a comment. changed unique_ptr to shared_ptr Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added a comment. In D67793#1678259 , @lawrence_danna wrote: > @labath > > I wrote a patch for the shared_ptr approach. It's simple, but it touches a > lot of lines. > > https://reviews.llvm.org/D67891 > > Now that I've gone and done it, I kind o

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. @labath I wrote a patch for the shared_ptr approach. It's simple, but it touches a lot of lines. https://reviews.llvm.org/D67891 Now that I've gone and done it, I kind of like it better that way. If you approve the other patch, I'll update this one accordin

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. @labath So what do you think? Update all the lldb_private::File clients to use shared_ptr instead? Or do it this way, with sharing handled inside of File? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna updated this revision to Diff 221113. lawrence_danna added a comment. added some tests, rebased Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67793/new/ https://reviews.llvm.org/D67793 Files: lldb/include/lldb/API/LLDB.h lldb/in

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna added a comment. In D67793#1676592 , @labath wrote: > The api seems reasonably straight-forward. The one thing that struck me is > that you're using unique_ptrs for the pimpled objects. That would mean the > SBFile objects are non-copyable

[Lldb-commits] [PATCH] D67793: new api class: SBFile

labath added a comment. This is an interesting undertaking. There's a lot of confusion about FILE* in lldb, and it would be great to clean some of that stuff up. The api seems reasonably straight-forward. The one thing that struck me is that you're using unique_ptrs for the pimpled objects. Tha

[Lldb-commits] [PATCH] D67793: new api class: SBFile

lawrence_danna created this revision. lawrence_danna added reviewers: JDevlieghere, jasonmolenda, zturner, jingham. Herald added a subscriber: mgorny. Herald added a project: LLDB. SBFile is a scripting API wrapper for lldb_private::File This is the first step in a rework of https://reviews.llvm.