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()..

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.

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

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.

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).

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

Logged In: YES 
user_id=971153

attaching the new patch

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!