Repository navigation
Step Into not listing targets properly #576
Description
Doing some testing I found an issue in how the targets are computed.
A construct such as:
def function():
yield sys._getframe()
bbb = foo.bar(
Something(param1, param2=xxx.yyy), {}
)
call()
Is not finding bar properly.
The main issue is that a stack is created and later when a CALL_FUNCTION bytecode is found items are popped from the stack, but the stack is incomplete, so, either the way to deal with the stack becomes much more complete (as we need to deal with much more bytecode to have a complete picture on how the stack would look -- which may not be worth it) or go with a simpler approach to just list the names without validating if those would actually be function calls (which may be reasonable since cases such as accessing properties would also work -- those don't work right now with the current approach).
I'm pending towards implementing the 2nd approach...
Activity
fabioz commentedon Apr 1, 2021
Pavel Minaev (@int19h) Karthik Nadig (@karthiknadig) the approach I had in mind doesn't work because the bytecode offset won't match.
i.e.: What the step into target currently does is check in a called frame that the back frame was the initial frame where the step into was requested and the bytecode offset matches the expected bytecode for the call. The
LOAD_XXXinstructions have the wrong offset to check that, so, the only way I can see how to fix this is actually properly implementing the stack operations to know which name matches a given call operation (or possibly aLOAD_ATTRto deal with@property).It'd be comparable to something as: https://github.com/nedbat/byterun/blob/master/byterun/pyvm2.py but only working at the metadata level, not really interpreting code.
So, I think this is going to take a while to implement (my guess is it'll take around 10-15 working days (which at my current time allocated means something around 2 months) as it needs to support many versions of python, which means lots of bytecodes to test). My plan is starting to work at that right now, but possibly pausing for smaller (or more critical) issues over time.
-- I'm also open to other approaches you may envision here.... another approach I thought of would be using the actual source code and creating an AST and then trying to do a match from which name loaded matches which called name to match the bytecode -- I think it'd be faster to implement, but I'm not 100% sure we can cover all cases this way and it wouldn't work on cases where sources aren't available.
fabioz commentedon Apr 1, 2021
Pavel Minaev (@int19h) Karthik Nadig (@karthiknadig) I think that if we could support only Python 3.5 onwards for this feature it'd be nicer to implement.
That would enable using https://github.com/MatthieuDartiailh/bytecode (which is already integrated into debugpy) as well as ignoring a bunch of Python 2.7 specific bytecode...
Do you think that's ok?
karthiknadig commentedon Apr 1, 2021
3.5+ for this feature is fine.
fabioz commentedon Apr 16, 2021
Note: I'm still working on this (it's a big feature and it'll still take a while to be complete).
In a proof of concept the new approach seems to be working reasonably well...
I also found an interesting corner case (which I'm also working on as a part of this issue), which is stepping into a function which is inside a generator -- in this case an intermediary frame such as
<listcomp>or<genexpr>is created, so, we need to stop based on 2 bytecode offsets, not a single one as the back frame is not immediately what'd be expected in the regular case.Improvements to step into target. Work in progress: microsoft#576
Improvements to step into target. Work in progress: microsoft#576
Improvements to step into target. Work in progress: microsoft#576
Improvements to step into target. Work in progress: #576
Improvements to step into target. Fixes microsoft#576
Improvements to step into target. Fixes microsoft#576
Improvements to step into target. Fixes microsoft#576