[1/2] GDB: S12Z: Add assertion

  ---
 gdb/s12z-tdep.c | 1 +
 1 file changed, 1 insertion(+)
  

On Mon, 12 Nov 2018 10:17:20 +0100
John Darrington <john@darrington.wattle.id.au> wrote:

> ---
>  gdb/s12z-tdep.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
> index 79f5772035..bd0bd7c001 100644
> --- a/gdb/s12z-tdep.c
> +++ b/gdb/s12z-tdep.c
> @@ -320,6 +320,7 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
>      }
>    else
>      {
> +      gdb_assert (this_sp == this_sp_for_id);
>        /* The stack pointer of the prev frame is frame_size greater
>           than the stack pointer of this frame plus one address
>           size (caused by the JSR or BSR).  */
> -- 

Could you please add a ChangeLog entry?

Also, on the same topic, what happened to the ChangeLog entries
for the initial commit / push of the s12z port?  (I don't see them
in the ChangeLog file.)

As for adding the assert, I suppose that you're just reminding the
reader of this earlier assignment? ...

  this_sp_for_id = this_sp;

(I didn't see anything which might change either this_sp or
this_sp_for_id along the code path leading to the assert.)

Kevin
  

On Sat, Nov 17, 2018 at 02:59:28PM -0700, Kevin Buettner wrote:
     
     > ---
     >  gdb/s12z-tdep.c | 1 +
     >  1 file changed, 1 insertion(+)
     > 
     > diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
     > index 79f5772035..bd0bd7c001 100644
     > --- a/gdb/s12z-tdep.c
     > +++ b/gdb/s12z-tdep.c
     > @@ -320,6 +320,7 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
     >      }
     >    else
     >      {
     > +      gdb_assert (this_sp == this_sp_for_id);
     >        /* The stack pointer of the prev frame is frame_size greater
     >           than the stack pointer of this frame plus one address
     >           size (caused by the JSR or BSR).  */
     > -- 
     
     Could you please add a ChangeLog entry?
     
     Also, on the same topic, what happened to the ChangeLog entries
     for the initial commit / push of the s12z port?  (I don't see them
     in the ChangeLog file.)

Is it really necessary in 2018 to have ChangeLogs?  After all, the same 
information is available by typing git log and/or git diff.  If the answer is 
"yes", then how about generating it automatically using the git-log-to-changelog
script which is available in gnulib ?  Many other projects have gone over to 
this method.

ChangeLogs are a dreaded nuisance.  They are almost guaranteed to cause merge 
conflicts.  The information they contain is redundant.  Their formatting 
requirements are onerous.  And in thirty years of software development, I don't 
think I've ever experienced a case where I've needed to refer to one.
     
     As for adding the assert, I suppose that you're just reminding the
     reader of this earlier assignment? ...
     
       this_sp_for_id = this_sp;

Indeed.  
     
     (I didn't see anything which might change either this_sp or
     this_sp_for_id along the code path leading to the assert.)

I have some plans for future changes in this part of the code.

J'
  

On Sun, 18 Nov 2018 10:22:58 +0100
John Darrington <john@darrington.wattle.id.au> wrote:

> On Sat, Nov 17, 2018 at 02:59:28PM -0700, Kevin Buettner wrote:
>      
>      > ---
>      >  gdb/s12z-tdep.c | 1 +
>      >  1 file changed, 1 insertion(+)
>      > 
>      > diff --git a/gdb/s12z-tdep.c b/gdb/s12z-tdep.c
>      > index 79f5772035..bd0bd7c001 100644
>      > --- a/gdb/s12z-tdep.c
>      > +++ b/gdb/s12z-tdep.c
>      > @@ -320,6 +320,7 @@ s12z_frame_cache (struct frame_info *this_frame, void **prologue_cache)
>      >      }
>      >    else
>      >      {
>      > +      gdb_assert (this_sp == this_sp_for_id);
>      >        /* The stack pointer of the prev frame is frame_size greater
>      >           than the stack pointer of this frame plus one address
>      >           size (caused by the JSR or BSR).  */
>      > --   
>      
>      Could you please add a ChangeLog entry?
>      
>      Also, on the same topic, what happened to the ChangeLog entries
>      for the initial commit / push of the s12z port?  (I don't see them
>      in the ChangeLog file.)
> 
> Is it really necessary in 2018 to have ChangeLogs?  After all, the same 
> information is available by typing git log and/or git diff.  If the answer is 
> "yes", then how about generating it automatically using the git-log-to-changelog
> script which is available in gnulib ?  Many other projects have gone over to 
> this method.
> 
> ChangeLogs are a dreaded nuisance.  They are almost guaranteed to cause merge 
> conflicts.  The information they contain is redundant.  Their formatting 
> requirements are onerous.  And in thirty years of software development, I don't 
> think I've ever experienced a case where I've needed to refer to one.

While I agree with some of your arguments against the use of
ChangeLogs in 2018, the GDB project still requires the use of
ChangeLogs.  See:

https://sourceware.org/gdb/wiki/ContributionChecklist#ChangeLog

and:

https://sourceware.org/gdb/wiki/ContributionChecklist#Properly_Formatted_GNU_ChangeLog

And, FWIW, the first thing that I did while reviewing your patches was
to look at the current ChangeLog to get a brief history of changes to
s12z-tdep.c. I was surprised when I did not find any ChangeLog entries,
though I did figure it out by consulting the git log.

Kevin
  

Hey John,

To add to what Kevin said, we've been discussing this topic many times.
I am pretty sure most people would be very happy to be getting rid of
ChangeLog entries. I think the main issue, now, is to automate their
generation when producing the source packages and releases; this is
because we are a GNU project, and we are expected to allow people
without the git repository to have a sense of the changes that were
made. I think that if we have a process in place that automates this,
we should be OK to replace the ChangeLog files by data extracted from
the revision logs.

What we need, at this point, is someone sufficiently motivated to
put in place this automatic generation. We might also need a bit of
development work on the hooks to validate the revision log's format
and confirm that it always contains a ChangeLog entry in a format
we can understand so we can always extract it (I can take care of
that part once we have an agreement on the process).