[PATCH] Minor reformatting in infrun.c

[PATCH] Minor reformatting in infrun.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 325 bytes --]

I am about to touch this part of the code, and it needed a bit of
reformatting, so I thought I'd commit the reformatting separately.

2002-08-16  Joel Brobecker  <brobecker@gnat.com>

        * infrun.c (handle_inferior_event): Minor reformatting, to make
        a rather long condition expression easier to read.

-- 
Joel

[-- Attachment #2: infrun.c.diff --]
[-- Type: text/plain, Size: 2261 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.65
diff -c -3 -p -r1.65 infrun.c
*** infrun.c	16 Aug 2002 17:56:17 -0000	1.65
--- infrun.c	16 Aug 2002 23:57:51 -0000
*************** handle_inferior_event (struct execution_
*** 2106,2125 ****
  	     singlestepping a trap instruction, and singlestepping thru a
  	     jump to the instruction following a trap instruction. */
  
! 	  stop_bpstat = bpstat_stop_status (&stop_pc,
! 					    /* Pass TRUE if our reason for stopping is something other
! 					       than hitting a breakpoint.  We do this by checking that
! 					       1) stepping is going on and 2) we didn't hit a breakpoint
! 					       in a signal handler without an intervening stop in
! 					       sigtramp, which is detected by a new stack pointer value
! 					       below any usual function calling stack adjustments.  */
! 					    (currently_stepping (ecs)
! 					     && prev_pc !=
! 					     stop_pc - DECR_PC_AFTER_BREAK
! 					     && !(step_range_end
! 						  && INNER_THAN (read_sp (),
! 								 (step_sp -
! 								  16)))));
  	  /* Following in case break condition called a
  	     function.  */
  	  stop_print_frame = 1;
--- 2106,2124 ----
  	     singlestepping a trap instruction, and singlestepping thru a
  	     jump to the instruction following a trap instruction. */
  
! 	  stop_bpstat =
!             bpstat_stop_status
!               (&stop_pc,
!                /* Pass TRUE if our reason for stopping is something other
!                   than hitting a breakpoint.  We do this by checking that
!                   1) stepping is going on and 2) we didn't hit a breakpoint
!                   in a signal handler without an intervening stop in
!                   sigtramp, which is detected by a new stack pointer value
!                   below any usual function calling stack adjustments.  */
!                (currently_stepping (ecs)
!                 && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
!                 && !(step_range_end
!                      && INNER_THAN (read_sp (), (step_sp - 16)))));
  	  /* Following in case break condition called a
  	     function.  */
  	  stop_print_frame = 1;

Re: [PATCH] Minor reformatting in infrun.c

[Michael Snyder]

Joel Brobecker wrote:
> 
> I am about to touch this part of the code, and it needed a bit of
> reformatting, so I thought I'd commit the reformatting separately.
> 
> 2002-08-16  Joel Brobecker  <brobecker@gnat.com>
> 
>         * infrun.c (handle_inferior_event): Minor reformatting, to make
>         a rather long condition expression easier to read.
> 
> --
> Joel


Joel, 

I hate long lines as much as anybody, but I hate defeating indent
even more.  The middle part of this is just going to go back to 
the right again, the first time indent gets run.

You might prefer to have it happen under your control...

Re: [PATCH] Minor reformatting in infrun.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

> I hate long lines as much as anybody, but I hate defeating indent
> even more.  The middle part of this is just going to go back to 
> the right again, the first time indent gets run.
> 
> You might prefer to have it happen under your control...

Argh! That's right! Thanks.

After adding the followig dummy comment, the "indent"-ed version stays
very close to the hand-formatted version. Should I check it in?
(note, the attached patch does not include the changes introduced after
running gdb_indent).

2002-08-22  Joel Brobecker  <brobecker@gnat.com>

        * infrun.c (handle_inferior_event): Add a dummy comment to
        help indent find a more readable formatting of this line of code.

Thanks,
-- 
Joel

[-- Attachment #2: infrun.c.diff --]
[-- Type: text/plain, Size: 991 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.68
diff -c -3 -p -r1.68 infrun.c
*** infrun.c	20 Aug 2002 23:01:29 -0000	1.68
--- infrun.c	22 Aug 2002 23:31:08 -0000
*************** handle_inferior_event (struct execution_
*** 2113,2119 ****
  	     jump to the instruction following a trap instruction. */
  
  	  stop_bpstat =
!             bpstat_stop_status
                (&stop_pc,
                 /* Pass TRUE if our reason for stopping is something other
                    than hitting a breakpoint.  We do this by checking that
--- 2113,2119 ----
  	     jump to the instruction following a trap instruction. */
  
  	  stop_bpstat =
!             bpstat_stop_status  /* Force indent line break here */
                (&stop_pc,
                 /* Pass TRUE if our reason for stopping is something other
                    than hitting a breakpoint.  We do this by checking that

Re: [PATCH] Minor reformatting in infrun.c

[Michael Snyder]

Joel Brobecker wrote:
> 
> > I hate long lines as much as anybody, but I hate defeating indent
> > even more.  The middle part of this is just going to go back to
> > the right again, the first time indent gets run.
> >
> > You might prefer to have it happen under your control...
> 
> Argh! That's right! Thanks.
> 
> After adding the followig dummy comment, the "indent"-ed version stays
> very close to the hand-formatted version. Should I check it in?
> (note, the attached patch does not include the changes introduced after
> running gdb_indent).

Err, I was actually meaning to say "let's conform with indent", 
rather than "let's defeat indent".  How about moving the comment
outside the argument list?  Then the args can stay as they were.

> 
> 2002-08-22  Joel Brobecker  <brobecker@gnat.com>
> 
>         * infrun.c (handle_inferior_event): Add a dummy comment to
>         help indent find a more readable formatting of this line of code.
> 
> Thanks,
> --
> Joel
> 
>   ------------------------------------------------------------------------
> 
>    infrun.c.diffName: infrun.c.diff
>                 Type: Plain Text (text/plain)

Re: [PATCH] Minor reformatting in infrun.c

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 851 bytes --]

> Err, I was actually meaning to say "let's conform with indent", 
> rather than "let's defeat indent".  How about moving the comment
> outside the argument list?  Then the args can stay as they were.

I see what you mean now. I should have tried it before, instead of
assuming that moving the comment up would not prevent indent from
formatting the args back to the same previous unreadable way (I did not
realize that the comment had this effect in the formatting result)...

I can commit the following patch. I verified that a later re-indent
does not cause a formatting change in the second argument.

2002-08-23  Joel Brobecker  <brobecker@gnat.com>

        * infrun.c (handle_inferior_event): Move a comment outside of a
        function call, in order to avoid indent reformatting this part
        of the code in an unreadable way.

-- 
Joel

[-- Attachment #2: infrun.c.diff --]
[-- Type: text/plain, Size: 2392 bytes --]

Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.68
diff -c -3 -p -r1.68 infrun.c
*** infrun.c	20 Aug 2002 23:01:29 -0000	1.68
--- infrun.c	23 Aug 2002 16:17:49 -0000
*************** handle_inferior_event (struct execution_
*** 2110,2127 ****
  	     This is only important on targets where DECR_PC_AFTER_BREAK
  	     is non-zero.  The prev_pc test is meant to distinguish between
  	     singlestepping a trap instruction, and singlestepping thru a
! 	     jump to the instruction following a trap instruction. */
  
  	  stop_bpstat =
              bpstat_stop_status
                (&stop_pc,
-                /* Pass TRUE if our reason for stopping is something other
-                   than hitting a breakpoint.  We do this by checking that
-                   either we detected earlier a software single step trap or
-                   1) stepping is going on and 2) we didn't hit a breakpoint
-                   in a signal handler without an intervening stop in
-                   sigtramp, which is detected by a new stack pointer value
-                   below any usual function calling stack adjustments.  */
                 sw_single_step_trap_p
                 || (currently_stepping (ecs)
                     && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
--- 2110,2127 ----
  	     This is only important on targets where DECR_PC_AFTER_BREAK
  	     is non-zero.  The prev_pc test is meant to distinguish between
  	     singlestepping a trap instruction, and singlestepping thru a
! 	     jump to the instruction following a trap instruction.
  
+            Therefore, pass TRUE if our reason for stopping is
+            something other than hitting a breakpoint.  We do this by
+            checking that either: we detected earlier a software single
+            step trap or, 1) stepping is going on and 2) we didn't hit
+            a breakpoint in a signal handler without an intervening stop
+            in sigtramp, which is detected by a new stack pointer value
+            below any usual function calling stack adjustments.  */
  	  stop_bpstat =
              bpstat_stop_status
                (&stop_pc,
                 sw_single_step_trap_p
                 || (currently_stepping (ecs)
                     && prev_pc != stop_pc - DECR_PC_AFTER_BREAK

Re: [PATCH] Minor reformatting in infrun.c

[Michael Snyder]

Joel Brobecker wrote:
> 
> > Err, I was actually meaning to say "let's conform with indent",
> > rather than "let's defeat indent".  How about moving the comment
> > outside the argument list?  Then the args can stay as they were.
> 
> I see what you mean now. I should have tried it before, instead of
> assuming that moving the comment up would not prevent indent from
> formatting the args back to the same previous unreadable way (I did not
> realize that the comment had this effect in the formatting result)...
> 
> I can commit the following patch. I verified that a later re-indent
> does not cause a formatting change in the second argument.

Yes, and now I can see no reason not to put the arguments back
the way they were originally.  Or, say. 

  stop_bpstat =
    bpstat_stop_status (&stop_pc, 
                        [...]

I think it's more natural and readable than moving the paren down.

> 2002-08-23  Joel Brobecker  <brobecker@gnat.com>
> 
>         * infrun.c (handle_inferior_event): Move a comment outside of a
>         function call, in order to avoid indent reformatting this part
>         of the code in an unreadable way.
> 
> --
> Joel
> 
>   ------------------------------------------------------------------------
> 
>    infrun.c.diffName: infrun.c.diff
>                 Type: Plain Text (text/plain)

Re: [PATCH] Minor reformatting in infrun.c

[Joel Brobecker]

> > I can commit the following patch. I verified that a later re-indent
> > does not cause a formatting change in the second argument.
> 
> Yes,

I committed this change. 

> and now I can see no reason not to put the arguments back
> the way they were originally.  Or, say. 
> 
>   stop_bpstat =
>     bpstat_stop_status (&stop_pc, 
>                         [...]

Absolutely. But unfortunately this is not the way indent formats it:

<<
          stop_bpstat =
            bpstat_stop_status
            (&stop_pc,
             sw_single_step_trap_p
             || (currently_stepping (ecs)
                 && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
                 && !(step_range_end
                      && INNER_THAN (read_sp (), (step_sp - 16)))));
>>

Should I check-in the indent'ed version anyway? (I would send a separate
[PATCH] message)

-- 
Joel

Re: [PATCH] Minor reformatting in infrun.c

[Andrew Cagney]

> 
> Absolutely. But unfortunately this is not the way indent formats it:
> 
> <<
>           stop_bpstat =
>             bpstat_stop_status
>             (&stop_pc,
>              sw_single_step_trap_p
>              || (currently_stepping (ecs)
>                  && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
>                  && !(step_range_end
>                       && INNER_THAN (read_sp (), (step_sp - 16)))));
> 

Why not just do:

int not_a_sw_breakpoint = ......
stop_bpstat = ..., not_a_sw_breakpoint, ...

the expression is unreasonably complex anyway :-)

enjoy,
Andrew

Re: [PATCH] Minor reformatting in infrun.c

[Michael Snyder]

Joel Brobecker wrote:
> 
> > > I can commit the following patch. I verified that a later re-indent
> > > does not cause a formatting change in the second argument.
> >
> > Yes,
> 
> I committed this change.
> 
> > and now I can see no reason not to put the arguments back
> > the way they were originally.  Or, say.
> >
> >   stop_bpstat =
> >     bpstat_stop_status (&stop_pc,
> >                         [...]
> 
> Absolutely. But unfortunately this is not the way indent formats it:
> 
> <<
>           stop_bpstat =
>             bpstat_stop_status
>             (&stop_pc,
>              sw_single_step_trap_p
>              || (currently_stepping (ecs)
>                  && prev_pc != stop_pc - DECR_PC_AFTER_BREAK
>                  && !(step_range_end
>                       && INNER_THAN (read_sp (), (step_sp - 16)))));
> >>
> 
> Should I check-in the indent'ed version anyway? (I would send a separate
> [PATCH] message)

Nah, I guess we've wasted enough cycles worrying about this 
rather trivial change.  ;-)