BINDING_STACK_SIZE => SPECPDL_INDEX

BINDING_STACK_SIZE => SPECPDL_INDEX

[Richard Stallman]

The macro BINDING_STACK_SIZE was introduced for
specpdl_ptr - specpdl, but its name is misleading.
The value is not the *size* of the stack, but rather
the position of the stack pointer within the stack.

A better name would be SPECPDL_INDEX.

Would someone please make that global replacement?

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On Tue, 9 Jul 2002 12:52:10 -0600 (MDT), Richard Stallman <rms@gnu.org> wrote:

> A better name would be SPECPDL_INDEX.
> 
> Would someone please make that global replacement?

Done.


                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> On Tue, 9 Jul 2002 12:52:10 -0600 (MDT), Richard Stallman <rms@gnu.org> wrote:
> 
> > A better name would be SPECPDL_INDEX.
> > 
> > Would someone please make that global replacement?
> 
> Done.

For consistency, I suggest to change all occurrences of `specpdl_ptr -
specpdl' to SPECPDL_INDEX. 


Also the name of the variable where we store the value
of SPECPDL_INDEX is typically named `count' which I find pretty
confusing reading the code ... "count of what?".

I'd suggest changing the name of those variables to `specpdl_index' or
just `spix' (which isn't currently used anywhere) for short.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On 11 Jul 2002 15:27:33 +0200, storm@cua.dk (Kim F. Storm) wrote:

> For consistency, I suggest to change all occurrences of `specpdl_ptr -
> specpdl' to SPECPDL_INDEX. 

OK, I'll take care of it.

> I'd suggest changing the name of those variables to `specpdl_index' or
> just `spix' (which isn't currently used anywhere) for short.

"spix" seems a bit cryptic to me.


                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> On 11 Jul 2002 15:27:33 +0200, storm@cua.dk (Kim F. Storm) wrote:
> 
> > For consistency, I suggest to change all occurrences of `specpdl_ptr -
> > specpdl' to SPECPDL_INDEX. 
> 
> OK, I'll take care of it.
> 
> > I'd suggest changing the name of those variables to `specpdl_index' or
> > just `spix' (which isn't currently used anywhere) for short.
> 
> "spix" seems a bit cryptic to me.

Less cryptic that specpdl_index?  :-)

Actually, `specpdl_index' isn't really good either, as it is not the
current index, but the saved index.

Maybe one of these is better:
        old_specpdl
        saved_specpdl
        specpdl_save 
        specpdl_base
        base_specpdl
        specpdl_top


-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On 11 Jul 2002 16:48:02 +0200, no-spam@cua.dk (Kim F. Storm) wrote:

> Less cryptic that specpdl_index?  :-)

Well, er... yeah, I suppose... more or less. :-)

> Actually, `specpdl_index' isn't really good either, as it is not the
> current index, but the saved index.

Hm. I've blindly made the change, but now I have second thoughts.

In alloc.c, for example, there's that code:

  for (bind = specpdl; bind != specpdl_ptr; bind++)
    {
      mark_object (&bind->symbol);
      mark_object (&bind->old_value);
    }

Both specpdl and specpdl_ptr are pointers to the same structure, aren't
they? So specpdl_ptr - specpdl is really a count and not an index.

> Maybe one of these is better:
>         old_specpdl
>         saved_specpdl
>         specpdl_save 
>         specpdl_base
>         base_specpdl
>         specpdl_top

Assuming it is really an index and not a count, any one of those is
better, yes.


                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Andreas Schwab]

Juanma Barranquero <lektu@terra.es> writes:

|> In alloc.c, for example, there's that code:
|> 
|>   for (bind = specpdl; bind != specpdl_ptr; bind++)
|>     {
|>       mark_object (&bind->symbol);
|>       mark_object (&bind->old_value);
|>     }
|> 
|> Both specpdl and specpdl_ptr are pointers to the same structure, aren't
|> they? So specpdl_ptr - specpdl is really a count and not an index.

It is also an index into the specpdl array.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On Thu, 11 Jul 2002 17:48:01 +0200, Andreas Schwab <schwab@suse.de> wrote:

> It is also an index into the specpdl array.

Well, yeah, you're right. This is C we're talking 'bout, after all :)

Anyway, the switch to SPECPDL_INDEX is now done. Changing the variables
(count, speccount, speccount1, post_read_count, count1 and specpdl_count)
to use a single name seems sensible. I'll do it a bit at a time.

I'll use specpdl_top, if no one opposes it.

                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Stefan Monnier]

> On 11 Jul 2002 16:48:02 +0200, no-spam@cua.dk (Kim F. Storm) wrote:
> 
> > Less cryptic that specpdl_index?  :-)
> 
> Well, er... yeah, I suppose... more or less. :-)
> 
> > Actually, `specpdl_index' isn't really good either, as it is not the
> > current index, but the saved index.
> 
> Hm. I've blindly made the change, but now I have second thoughts.

Could people refrain from making such "gratuitous" pervasive changes ?
It's a pain in the ass to maintain local uncommitted changes when
you guys do that.


	Stefan

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On Thu, 11 Jul 2002 12:55:14 -0400, "Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> wrote:

> Could people refrain from making such "gratuitous" pervasive changes ?

Sorry. I changed BINDING_STACK_SIZE to SPECPDL_INDEX as per Richard's
request, and then it seemed logical to change all uses of specpdl_ptr -
specpdl to SPECPDL_INDEX.

                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Stefan Monnier]

> > Could people refrain from making such "gratuitous" pervasive changes ?
> 
> Sorry. I changed BINDING_STACK_SIZE to SPECPDL_INDEX as per Richard's

This is fine.  It's a new macro so it's OK to change it (code that uses
it is still fresh).

> request, and then it seemed logical to change all uses of specpdl_ptr -
> specpdl to SPECPDL_INDEX.

This might be a good change, so I can live with it: it's not gratuitous.
But the count -> index|spix|... switch is just plain silly and will only
make my life worse.


	Stefan

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

On Thu, 11 Jul 2002 13:08:58 -0400, "Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> wrote:

> But the count -> index|spix|... switch is just plain silly

Hmm... Not really. There are six or seven different names for the same
concept through the code. Unifying them would be useful, if only to make
understanding simpler for newcomers (like me).

But I suppose others will have to decide what to do in that case :)

                                                           /L/e/k/t/u

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Kim F. Storm]

Juanma Barranquero <lektu@terra.es> writes:

> On Thu, 11 Jul 2002 13:08:58 -0400, "Stefan Monnier" <monnier+gnu/emacs@rum.cs.yale.edu> wrote:
> 
> > But the count -> index|spix|... switch is just plain silly

Changes which can improve the maintainablility of the code are not silly.
You may not want them, and we may decide against them,  but that doesn't
make them silly.

It seems that some emacs hackers have a habit of keeping big thunks of
changes locally for a long time...   That's fine, but doing so should
not prevent others from maintaining the code in CVS.

> 
> Hmm... Not really. There are six or seven different names for the same
> concept through the code. Unifying them would be useful, if only to make
> understanding simpler for newcomers (like me).

Exactly my experience.

> 
> But I suppose others will have to decide what to do in that case :)
> 

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Richard Stallman]

Please don't rename the local variables called `count'.
That is a lot of change for very little benefit.

Re: BINDING_STACK_SIZE => SPECPDL_INDEX

[Juanma Barranquero]

> Please don't rename the local variables called `count'.

OK.

-- 
Juanma Barranquero <lektu@terra.es>