Issue1003640
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2004-08-04 23:23 by isandler, last changed 2022-04-11 14:56 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
pdb.patch2-v1 | isandler, 2004-08-04 23:23 | |||
pdb.patch2-v2 | isandler, 2004-08-25 00:53 |
Messages (8) | |||
---|---|---|---|
msg46529 - (view) | Author: Ilya Sandler (isandler) | Date: 2004-08-04 23:23 | |
This patch fixes the following bugs: #976878 PDB: unreliable breakpoints on functions #926369 pdb sets breakpoints in the wrong location #875404 global stmt causes breakpoints to be ignored Discussion: all 3 bugs are caused by faults in pdb.checkline() logic when it tries to determine the first executable line of a function body. checkline() did its job by doing basic parsing of the function body. The parsing was missing quite a few cases where checkline() returned either a wrong or non-executable line of code... The problem only happened when a breakpoint was set via function name: "b some_func" Solution: Instead of attempting to fix checkline() the patch instead changes the breakpoint logic as follows 1) When a breakpoint is set via a func tionname 1a) the bkpt gets the lineno of the def statement thus eliminating the parsing logic in checkline() 1b) a new funcname attribute is attached to the breakpoint 2) bdb.break_here() is changed to detect and handle 2 special cases 2a) def statement is executed...No breakpoint is needed 2b) a first executable line of a function with such a breakpoint is reached. Break is neded Overall line count in pdb+bdb has actually gone down slightly and I think this solution is cleaner than attempting to expand parsing in checkline().. |
|||
msg46530 - (view) | Author: Johannes Gijsbers (jlgijsbers) * | Date: 2004-08-13 15:13 | |
Logged In: YES user_id=469548 The patch works for me (and any patch that fixes three bugs should really be commited <wink>), but I have some suggestions: - The checkline() docstring needs to be updated. - Try not to use multi-line comments after a statement (e.g.: 'return False #it's not a func call, but rather', etc.). - For consistency, Breakpoint.breakingline should probably be called func_firstlineno. - Moving the 'bp.breakingline=frame.f_lineno' statement into an else: branch of the 'if bp.breakingline and (bp.breakingline != frame.f_lineno)' statement would probably make the logic clearer. |
|||
msg46531 - (view) | Author: Ilya Sandler (isandler) | Date: 2004-08-13 19:24 | |
Logged In: YES user_id=971153 >- The checkline() docstring needs to be updated. >- Try not to use multi-line comments after a statement >(e.g.: 'return False #it's not a func call, but rather', etc.). Ok, I can resubmit a patch for these two >- For consistency, Breakpoint.breakingline should probably >be called func_firstlineno. Well, I am not sure: func_firstlineno causes a wrong association with co_firstlineno (while these are almost never the same). Furthermore, the name ".breakingline" reflects the purpose: "a line number where to break", while, "func_firstlineno" is totatly neutral.. >- Moving the 'bp.breakingline=frame.f_lineno' statement into >an else: branch of the 'if bp.breakingline and >(bp.breakingline != frame.f_lineno)' statement would >probably make the logic clearer. Are you suggesting to replace: if bp.breakingline and (bp.breakingline != frame.f_lineno): return False bp.breakingline=frame.f_lineno with: if bp.breakingline and (bp.breakingline != frame.f_lineno): return False else: bp.breakingline=frame.f_lineno Why do you think it's better? It's definitely more verbose, has an extra indentation level and looking through the code I see a lot of code which looks like: if (....): return foo bar without the else: branch Would the following be better? if not bp.breakingline: #the function is entered for the 1st time bp.breakingline=frame.f_lineno if bp.breakingline != frame.f_lineno: return False Thus making it explicit that there are really 2 decisions being made: 1) do we need to set the breakingline 2) do we need to break here I don't have any strong feelings regarding these 2 issues, just want to make sure that I understand the problem |
|||
msg46532 - (view) | Author: Johannes Gijsbers (jlgijsbers) * | Date: 2004-08-14 10:40 | |
Logged In: YES user_id=469548 "Well, I am not sure: func_firstlineno causes a wrong association with co_firstlineno (while these are almost never the same)." Ah, I misread the patch there. How about func_first_executable_lineno (or something like that)? Or am I off-base again? "Would the following be better?" if not bp.breakingline: #the function is entered for the 1st time bp.breakingline=frame.f_lineno if bp.breakingline != frame.f_lineno: return False " Yes. I was just having a bit of a hard time following your patch. I think this is clearer. |
|||
msg46533 - (view) | Author: Ilya Sandler (isandler) | Date: 2004-08-15 03:30 | |
Logged In: YES user_id=971153 Ok, I'll submit an updated patch hopefully by the end of the next week (I will be out of town for the next few days). |
|||
msg46534 - (view) | Author: Ilya Sandler (isandler) | Date: 2004-08-22 16:32 | |
Logged In: YES user_id=971153 I am attaching a new version of the patch. In addition to issues raised earlier the v1 version of patch had a couple of other problems: 1. funcname based breakpoints got a wrong hit statistics (hit counter was incremented on every effective() call, which happened for every line of a function with a funcname bkpt) 2. If a user set a bkpt via line number at def statement (this is probably quite rare, but still possible) , then pdb would stop at every line of that function when the function is called To fix these I had to move most of funcname handling logic from break_here() into a separate function (bdb.checkfuncname) and call it from bdb.effective() Changes in the new version include: 1. Fix the issues with statistics and bkpts at def stmt 2. Fix a doc line of checkline() 3. Rename .breakingline into .func_first_executable_line 4. Comment fixes |
|||
msg46535 - (view) | Author: Ilya Sandler (isandler) | Date: 2004-08-25 00:53 | |
Logged In: YES user_id=971153 attaching the new patch |
|||
msg46536 - (view) | Author: Johannes Gijsbers (jlgijsbers) * | Date: 2004-08-30 13:31 | |
Logged In: YES user_id=469548 Checked in as rev 1.45 of bdb.py and rev 1.69 pdb.py with some modifications to comments and docstrings. Thanks for the patch! |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:06 | admin | set | github: 40688 |
2004-08-04 23:23:12 | isandler | create |