With lldb (& llvm) requiring Python 3.6+, use of the six module can be removed.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
lldb/packages/Python/lldbsuite/support/encoded_file.py | ||
---|---|---|
17–25 | this function only deviated in the case of python 2, so I removed it. | |
lldb/packages/Python/lldbsuite/support/seven.py | ||
12 | this function was not used externally, so I inlined it into get_command_output. | |
27 | this was also not used. | |
lldb/packages/Python/lldbsuite/test/lldbtest.py | ||
2278–2281 | this function had a parameter named str, which shadowed builtin.str. As a fix, in this file I renamed all variables named str to string. |
I left third_party/Python/module/six, in case there are any lldb scripts that depend on the existence of six.
Very cool! Thanks for taking care of this! LGTM with the 2 comments and assuming the test suite runs fine :)
lldb/examples/python/scripted_process/scripted_process.py | ||
---|---|---|
5 | nit: no need to specify metaclass=ABCMeta, it can just be ScriptedProcess(ABC) | |
194 | ditto | |
lldb/packages/Python/lldbsuite/test/lldbtest.py | ||
2278–2281 | nit: If you feel like it, may be you can split everything related to the str-> string refactor into a separate since it's orthogonal to removing six |
lldb/examples/python/scripted_process/scripted_process.py | ||
---|---|---|
5 | disregard this comment, that's actually the way to do it ... |
lldb/packages/Python/lldbsuite/test/lldbtest.py | ||
---|---|---|
2278–2281 | I need to at least make the str -> string rename in the functions where the refactor involves isinstance(..., str). I will remove the other renames, for another diff. |
lldb/bindings/python/CMakeLists.txt | ||
---|---|---|
63–68 ↗ | (On Diff #450843) | @DavidSpickett I removed LLDB_USE_SYSTEM_SIX but left the code that copies vendored copy of six.py. Is this what you had in mind? My thinking is there might be lldb scripts that use six, and I don't want the removal of internal use of six to be a breaking change downstream, not yet anyway. |
lldb/bindings/python/CMakeLists.txt | ||
---|---|---|
63–68 ↗ | (On Diff #450843) | I hadn't considered user scripts good point. If you were using this option, you'd now get a warning from cmake that it's value was unused. Would it make more sense to have a proper deprecation warning and remove it and the copy of six at some later point? Please land this change minus the LLDB_USE_SYSTEM_SIX stuff though. I don't want to block the rest on this. |
nit: no need to specify metaclass=ABCMeta, it can just be ScriptedProcess(ABC)