lawrence_danna marked 3 inline comments as done. lawrence_danna added a comment.
In D68995#1712387 <https://reviews.llvm.org/D68995#1712387>, @labath wrote: > In D68995#1711831 <https://reviews.llvm.org/D68995#1711831>, @lawrence_danna > wrote: > > > In D68995#1710594 <https://reviews.llvm.org/D68995#1710594>, @labath wrote: > > > > > Thanks for jumping onto this. Apart from the inline comments, I have one > > > high level question: What are the cases that the old method is wrong for? > > > Was it just builtin functions, or are there other cases too? Is it > > > possible to fix it to work for builtings too? (okay, those were three > > > questions) > > > > > > What I am wondering is how important it is to maintain two methods for > > > fetching the argument information. Builtin functions are not likely to be > > > used as lldb callbacks, so if it's just those, it may be sufficient to > > > just leave a TODO to use the new method once we are in a position to > > > require python>=3.3. > > > > > > Looks like the old implementation also doesn't work for class or static > > methods, or for objects with `__call__` method. I added a bunch of tests > > for script commands with various callable types in the followon patch. > > > Ok, I see... Would it be hard/impossible to fix that using the previous > method, so that we have a unified implementation? Or would that result in a > bunch of ifdefs too ? I'm sorry if that's obvious, but I don't know much > about the C python api.. I did fix the previous method for python2, at least for the callable types I could think of to test. There must be some way to get something resembling the old method to work for python3, because `inspect` does it. But it would be harder to get right, and a lot less likely to keep being correct in future versions of python. And I don't think it's possible to have a single implementation like that that works for both 2 and 3 without ifdefs. I was thinking that since python2 is EOL in like two months anyway, the best thing to do is to just keep the old implementation around until we drop python 2 support, and then delete it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68995/new/ https://reviews.llvm.org/D68995 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
