Re: Emacs-diffs Digest, Vol 94, Issue 89

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> === modified file 'src/scroll.c'
> --- a/src/scroll.c	2010-07-12 17:47:17 +0000
> +++ b/src/scroll.c	2010-09-24 15:20:58 +0000
> @@ -94,7 +94,7 @@
>  		     int free_at_end)
>  {
>    register int i, j;
> -  int frame_lines = FRAME_LINES (frame);
> +  EMACS_INT frame_lines = FRAME_LINES (frame);

Is this really a good idea?  There's no chance the number of lines in
a frame will overflow a 32-bit int any time soon.  SO I think an
explicit cast to int is a better solution here.

(There are other similar places in this revision.)

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Is this really a good idea?  There's no chance the number of lines in
> a frame will overflow a 32-bit int any time soon.  SO I think an
> explicit cast to int is a better solution here.

That was my first impulse, too, but the other int casting got negative feedback..

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Fri, 24 Sep 2010 21:41:24 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Is this really a good idea?  There's no chance the number of lines in
> > a frame will overflow a 32-bit int any time soon.  SO I think an
> > explicit cast to int is a better solution here.
> 
> That was my first impulse, too, but the other int casting got negative feedback..

You mean, GCC still complains?

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> Date: Fri, 24 Sep 2010 21:50:27 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> > Date: Fri, 24 Sep 2010 21:41:24 +0200
> > 
> > Eli Zaretskii <eliz@gnu.org> writes:
> > 
> > > Is this really a good idea?  There's no chance the number of lines in
> > > a frame will overflow a 32-bit int any time soon.  SO I think an
> > > explicit cast to int is a better solution here.
> > 
> > That was my first impulse, too, but the other int casting got negative feedback..
> 
> You mean, GCC still complains?

Or maybe you mean that Stefan thought casting was not the best idea.
In that case, I'd like Stefan's opinion on this one:

> -  int frame_lines = FRAME_LINES (frame);
> +  EMACS_INT frame_lines = FRAME_LINES (frame);

I think down that lane lies madness, because we have lots of struct
members that are Lisp integers, and assigning the result of XINT of
every one of them to an EMACS_INT will significantly and unnecessarily
increase our stack usage (on 64-bit machines).

Re: Emacs-diffs Digest, Vol 94, Issue 89

[David Kastrup]

Eli Zaretskii <eliz@gnu.org> writes:

> Or maybe you mean that Stefan thought casting was not the best idea.
> In that case, I'd like Stefan's opinion on this one:
>
>> -  int frame_lines = FRAME_LINES (frame);
>> +  EMACS_INT frame_lines = FRAME_LINES (frame);
>
> I think down that lane lies madness, because we have lots of struct
> members that are Lisp integers, and assigning the result of XINT of
> every one of them to an EMACS_INT will significantly and unnecessarily
> increase our stack usage (on 64-bit machines).

"Significantly"?  Reality check.  How deep are you planning to recurse?
Are you still trying to fit a working Emacs into 64kB?

The significant costs are, if at all, in the structs containing the
"struct members" since in contrast to the automatic scalar variables of
a typical function that does rarely if ever recurse, there can be lots
of them at once, and apparently those _are_ already EMACS_INT, so _that_
cost, which is likely quite more significant, has already been paid
without people complaining.

-- 
David Kastrup

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

>> -  int frame_lines = FRAME_LINES (frame);
>> +  EMACS_INT frame_lines = FRAME_LINES (frame);
>
> I think down that lane lies madness, because we have lots of struct
> members that are Lisp integers, and assigning the result of XINT of
> every one of them to an EMACS_INT will significantly and unnecessarily
> increase our stack usage (on 64-bit machines).

I think text_lines and related members of `struct frame' should be ints.
Dunno why they were made EMACS_INT, though it happened long ago, before
Emacs-21.

I doubt there is much practical difference, only a conceptual one; IMO
the "EMACS_INT" cast is a nice way to denote that an integer may have to
do with buffer and string sizes.

I don't feel strongly about it, though.

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

>> -  int frame_lines = FRAME_LINES (frame);
>> +  EMACS_INT frame_lines = FRAME_LINES (frame);

> Is this really a good idea?  There's no chance the number of lines in
> a frame will overflow a 32-bit int any time soon.  SO I think an
> explicit cast to int is a better solution here.

BTW, if you add casts, as much as possible add them only inside macros,


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> From: David Kastrup <dak@gnu.org>
> Date: Fri, 24 Sep 2010 22:29:02 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Or maybe you mean that Stefan thought casting was not the best idea.
> > In that case, I'd like Stefan's opinion on this one:
> >
> >> -  int frame_lines = FRAME_LINES (frame);
> >> +  EMACS_INT frame_lines = FRAME_LINES (frame);
> >
> > I think down that lane lies madness, because we have lots of struct
> > members that are Lisp integers, and assigning the result of XINT of
> > every one of them to an EMACS_INT will significantly and unnecessarily
> > increase our stack usage (on 64-bit machines).
> 
> "Significantly"?  Reality check.

Yes, indeed.  Take a look at GC some day, it uses up tens of thousands
of recursive calls as a matter of routine.

And even without GC, there are about 30 frames on the stack any time I
debug Emacs.  If we pacify -Wconversion by using EMACS_INT for every
variable that sometimes gets assigned the result of XINT or XFASTINT,
that could be quite a lot of memory waste.

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00
> 	autolearn=unavailable version=3.3.1
> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 25 Sep 2010 00:25:10 +0200
> Cc: Lars Magne Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> 
> >> -  int frame_lines = FRAME_LINES (frame);
> >> +  EMACS_INT frame_lines = FRAME_LINES (frame);
> 
> > Is this really a good idea?  There's no chance the number of lines in
> > a frame will overflow a 32-bit int any time soon.  SO I think an
> > explicit cast to int is a better solution here.
> 
> BTW, if you add casts, as much as possible add them only inside macros,

So what is the practical guidance to Lars and others who go by the
warnings emitted by -Wconversion?  Will the below be okay, for when an
small-valued EMACS_INT is assigned to an int:

 . if the source is EMACS_INT, consider making it an int; if that's
   not possible, cast to int, preferably inside a macro

 . if the source is a Lisp_Object passed through XINT, cast to an int,
   preferably inside an accessor macro

Re: Emacs-diffs Digest, Vol 94, Issue 89

[David Kastrup]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Fri, 24 Sep 2010 22:29:02 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Or maybe you mean that Stefan thought casting was not the best idea.
>> > In that case, I'd like Stefan's opinion on this one:
>> >
>> >> -  int frame_lines = FRAME_LINES (frame);
>> >> +  EMACS_INT frame_lines = FRAME_LINES (frame);
>> >
>> > I think down that lane lies madness, because we have lots of struct
>> > members that are Lisp integers, and assigning the result of XINT of
>> > every one of them to an EMACS_INT will significantly and unnecessarily
>> > increase our stack usage (on 64-bit machines).
>> 
>> "Significantly"?  Reality check.
>
> Yes, indeed.  Take a look at GC some day, it uses up tens of thousands
> of recursive calls as a matter of routine.

And how many of those garbage collection calls are involved in this
particular issue?  About zero I should guess.

> And even without GC, there are about 30 frames on the stack any time I
> debug Emacs.

Of which there are, pessimistically speaking, probably 10 affected.  So
we are talking about several dozens of wasted bytes at any given point
of time.  Very much dwarved by the stack requirements of garbage
collection, according to you.

> If we pacify -Wconversion by using EMACS_INT for every variable that
> sometimes gets assigned the result of XINT or XFASTINT, that could be
> quite a lot of memory waste.

We are talking about automatic variables here.  When optimizing, the
compiler does not even reserve stack space when it does not need it.  In
fact, you are likely getting code bloat if you force intermediate values
into variables of smaller size than the original type.  The compiler
works with 64-bit registers in the affected architectures, anyway, and
it needs to fiddle with masking/extending operations in order to
simulate 32-bit variables.  It generates extra code in order to produce
the behavior that the compiler warns about.

-- 
David Kastrup

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> >> -  int frame_lines = FRAME_LINES (frame);
>> >> +  EMACS_INT frame_lines = FRAME_LINES (frame);

[...]

> So what is the practical guidance to Lars and others who go by the
> warnings emitted by -Wconversion?  Will the below be okay, for when an
> small-valued EMACS_INT is assigned to an int:
>
>  . if the source is EMACS_INT, consider making it an int; if that's
>    not possible, cast to int, preferably inside a macro
>
>  . if the source is a Lisp_Object passed through XINT, cast to an int,
>    preferably inside an accessor macro

So in this case FRAME_LINES should cast to an int before returning?
Makes sense to me, and I can redo the stuff in that file based on that. 

Or just make these into ints:

  /* Size of this frame, excluding fringes, scroll bars etc.,
     in units of canonical characters.  */
  EMACS_INT text_lines, text_cols;

It does seem rather nonsensical to have them be EMACS_INT...

Which reminds me about a question about characters.  Can they really be
longs?  Let's see...

DEFUN ("char-resolve-modifiers", Fchar_resolve_modifiers,
...
  (Lisp_Object character)
{
  int c;

  CHECK_NUMBER (character);
  c = XINT (character);
  return make_number (char_resolve_modifier_mask (c));
}

Hm.  In that case it should perhaps actually be EMACS_INT, since the
character comes from "user space".  So that's a bad example.  :-)  But
the general question is whether characters can be longs still stand, and
whether there then should be like an XCHAR macro that does the same as
XINT, but casts to int.  

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 25 Sep 2010 15:19:37 +0200
> 
> So in this case FRAME_LINES should cast to an int before returning?
> Makes sense to me, and I can redo the stuff in that file based on that. 
> 
> Or just make these into ints:
> 
>   /* Size of this frame, excluding fringes, scroll bars etc.,
>      in units of canonical characters.  */
>   EMACS_INT text_lines, text_cols;

Given the controversy, I'll defer to Stefan and Chong for a definitive
guidance.

> Which reminds me about a question about characters.  Can they really be
> longs?

AFAIK, characters in Emacs are presently 32-bit entities, so an int
should suffice.

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

> I think text_lines and related members of `struct frame' should be ints.

Agreed.


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

>  . if the source is a Lisp_Object passed through XINT, cast to an int,
>    preferably inside an accessor macro

Ideally, we'd have two XINT-style macros, one that returns an int and
one that returns an EMACS_INT.  E.g. for chars, we'd use the `int'
version since chars use less than 32 bits.


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Ideally, we'd have two XINT-style macros, one that returns an int and
> one that returns an EMACS_INT.  E.g. for chars, we'd use the `int'
> version since chars use less than 32 bits.

Makes sense.  The other macro could just be a wrapper around XINT that
coerces to int.

What would it be called?  XIINT?  Eek.  Uhm.  XNINT?  (For "not long
int.")  :-)  Ok, I have nothing.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> From: Lars Magne Ingebrigtsen <larsi@gnus.org>
> Date: Sat, 25 Sep 2010 22:30:09 +0200
> 
> What would it be called?  XIINT?  Eek.  Uhm.  XNINT?  (For "not long
> int.")  :-)  Ok, I have nothing.

IXINT?

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

>> What would it be called?  XIINT?  Eek.  Uhm.  XNINT?  (For "not long
>> int.")  :-)  Ok, I have nothing.
> IXINT?

XINT would be the obvious name, but it's already taken.
IXINT doesn't follow the "X..." convention we've used so far, but
I won't oppose it.  We could go with XINT_int, otherwise.

Another question is whether it should do range-checking or just silent
wrap-around.


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Chong Yidong]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> What would it be called?  XIINT?  Eek.  Uhm.  XNINT?  (For "not long
>>> int.")  :-)  Ok, I have nothing.
>> IXINT?
>
> XINT would be the obvious name, but it's already taken.
> IXINT doesn't follow the "X..." convention we've used so far, but
> I won't oppose it.  We could go with XINT_int, otherwise.

How about XINT for ints and XEINT for EMACS_INTs?

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Chong Yidong <cyd@stupidchicken.com> writes:

> How about XINT for ints and XEINT for EMACS_INTs?

So rename all instances of XINT in the source tree to XEINT first?

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

>> How about XINT for ints and XEINT for EMACS_INTs?
> So rename all instances of XINT in the source tree to XEINT first?

That's the part I didn't like about using XINT for `int'.


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> So rename all instances of XINT in the source tree to XEINT first?
>
> That's the part I didn't like about using XINT for `int'.

Me neither.  The code churn would be massive, wouldn't it?

I think IXINT is the most pleasing suggestion so far.

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Andreas Schwab]

XSMALLINT

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Date: Mon, 27 Sep 2010 23:21:16 +0200
> 
> XSMALLINT

XINT2int

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Lars Magne Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> XSMALLINT
>
> XINT2int

XINTINT

-- 
(domestic pets only, the antidote for overdose, milk.)
  larsi@gnus.org * Lars Magne Ingebrigtsen

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Stefan Monnier]

>>> XSMALLINT
>> XINT2int
> XINTINT

Nobody cares: pick one.


        Stefan

Re: Emacs-diffs Digest, Vol 94, Issue 89

[Davis Herring]

>> XSMALLINT
>
> XINT2int

If we're allowing lowercase, we could just use Xint.  Or, if we need
"mostly" capitals, XiNT or XCint.

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.