GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-59013: Make line number of function breakpoint more precise by gaogaotiantian · Pull Request #110582 · python/cpython

Conversation

@gaogaotiantian

@gaogaotiantian gaogaotiantian commented Oct 9, 2023 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Currently if you set a breakpoint on a function like break foo, it will claim that it sets a breakpoint on the line the function is defined (aka def foo()). However, if you set a breakpoint using line number (break 4 for example), even if the description of the breakpoint is exactly the same, they have different behaviors.

Actually, when we set a breakpoint on a function, we did not set the breakpoint to the line it is defined, because normally there's no executable code on that line. The first line we would stop, is the first executable line in that function.

This patch uses a heuristic - to find the line number of the first instruction that is not RESUME. If failed, fall back to co_firstlineno.

This should cover almost all cases (I can't think of outliners, but maybe there is), and won't give a worse result than before.

  • Issue: Pdb does not stop at a breakpoint set on the line of a function definition #59013

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
gaogaotiantian added 2 commits October 9, 2023 14:36
@gaogaotiantian
Make line number of function breakpoint more precise
5485aee
@gaogaotiantian
Check None for lineno
fb467eb
@bedevere-app bedevere-app Bot added the awaiting review label Oct 9, 2023
@bedevere-app bedevere-app Bot mentioned this pull request Oct 9, 2023
Pdb does not stop at a breakpoint set on the line of a function definition #59013
Closed
@gaogaotiantian
gaogaotiantian requested a review from iritkatriel October 9, 2023 21:43
@blurb-it
📜🤖 Added by blurb_it.
80322d0
iritkatriel
iritkatriel reviewed Oct 10, 2023
View reviewed changes
Comment thread Lib/pdb.py Outdated
Return code.co_firstlineno if no executable line is found.
"""
for instr in dis.get_instructions(code):
if instr.opname != 'RESUME' and instr.positions.lineno is not None:

@iritkatriel iritkatriel Oct 10, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might not always work:

>>> def f():
...    yield 42
... 
>>> dis.dis(f)
   1           0 RETURN_GENERATOR

None           2 POP_TOP

   1           4 RESUME                   0

   2           6 LOAD_CONST               1 (42)
               8 YIELD_VALUE              1
              10 RESUME                   1
              12 POP_TOP
              14 RETURN_CONST             0 (None)

None     >>   16 CALL_INTRINSIC_1         3 (INTRINSIC_STOPITERATION_ERROR)
              18 RERAISE                  1
ExceptionTable:
  4 to 14 -> 16 [0] lasti
>>> 

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gaogaotiantian gaogaotiantian Oct 11, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True - it's not "worse" than the current solution though.

Actually would it be more reasonable to use the line number of the instruction after RESUME?

Now that I think about it, generators probabaly have more problems with function breakpoints - when I set a breakpoint on a generator function, I'd hope that the breakpoint is hit every time the function is entered right? And pdb is not able to do that now - it stores the line that executed first and break on that line. We could potentially enter the generator on a different line. So the problem is more serious already on generators.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Oct 11, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually would it be more reasonable to use the line number of the instruction after RESUME?

I think the code object already has that in a field called _co_firsttraceable.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Oct 11, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I think about it, generators probabaly have more problems with function breakpoints

I can believe that. When you call a generator function it creates a generator object and returns it. Then you repeatedly call the generator object (which executes the same code, past the point of the RETURN_GENERATOR).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gaogaotiantian gaogaotiantian Oct 11, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the code object already has that in a field called _co_firsttraceable.

Yes and I don't think it was exposed to Python level.

I can believe that. When you call a generator function it creates a generator object and returns it. Then you repeatedly call the generator object (which executes the same code, past the point of the RETURN_GENERATOR).

I caused an assertion error when I was trying to test a little bit more with generators - I'll investigate into it.

From a user's point, what would be the expected behavior if they set a breakpoint to the generator function? Do they want a break when the generator is being created (so actually RETURN_GENERATOR)? That's a valid call. Or do they want a break when the "first time" the generator is executed? Or every time the generator is executed? Those are three different behaviors and the third one has issues with display the line number.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Oct 11, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect it to be the first time the generator is executed. The next time it will start executing after some yield, and that's not the first line of the function.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gaogaotiantian gaogaotiantian Oct 11, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we can use the RESUME method I mentioned above (which I think is basically how _co_firsttraceable works). Or do you think we should expose that member to Python?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@iritkatriel iritkatriel Oct 11, 2023

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can re-implement it. As long as we have a test that will give us a heads up when it needs to change it should be ok.

@markshannon do you agree?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@gaogaotiantian gaogaotiantian Oct 12, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the line searching method to use the instruction after RESUME. Also a generator test case is added.

There is one thing that I realized - if you do break func before the func is defined(evaluated), you'll still get a line number at the function definition - it uses re to find the function.

Is it better to have a consistant wrong answer, or a partially correct one?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
gaogaotiantian added 2 commits October 11, 2023 18:41
@gaogaotiantian
Merge branch 'main' into pdb-func-break
b6d95e3
@gaogaotiantian
Add generator test and use instr after RESUME
3955333
@gaogaotiantian

gaogaotiantian commented Oct 27, 2023

Copy link
Copy Markdown
Member Author

@markshannon @iritkatriel do you have any feedback on this PR? Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

iritkatriel
iritkatriel approved these changes Oct 27, 2023
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Oct 27, 2023
@iritkatriel
iritkatriel merged commit 1c9a0c4 into python:main Oct 27, 2023
@bedevere-app bedevere-app Bot removed the awaiting merge label Oct 27, 2023
@gaogaotiantian
gaogaotiantian deleted the pdb-func-break branch October 27, 2023 21:44
iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request Oct 29, 2023
@gaogaotiantian @iritkatriel
pythongh-59013: Make line number of function breakpoint more precise (p…
3aeffca
…ython#110582)
@gaogaotiantian gaogaotiantian mentioned this pull request Nov 27, 2023
gh-59013: Set breakpoint on the first executable line in pdb when using break func #112470
Merged
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@gaogaotiantian @aisk
pythongh-59013: Make line number of function breakpoint more precise (p…
5c271de
…ython#110582)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@gaogaotiantian @Glyphack
pythongh-59013: Make line number of function breakpoint more precise (p…
a3b8768
…ython#110582)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iritkatriel iritkatriel iritkatriel approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@gaogaotiantian @iritkatriel
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.