(unknown)

(unknown)

[Eli Zaretskii]

Dmitry, could you please explain the reason(s) for the change below?

Vertical positions in a window can never be large enough to justify
using ptrdiff_t (on platforms where that is wider than a 32-bit
'int').  These are pixel positions on the Emacs display, so they
cannot be too large.  The type of 'first_vpos' looks especially
strange, since it is explicitly set to 1 at most.  I'm not aware of a
platform where an 'int' is not wide enough for a value 1 ;-)

Using inappropriate data types makes the code harder to read and
understand, because it hints that something non-obvious is going on
somewhere.

	 * xdisp.c (try_window_id): Change type of 'first_vpos' and 'vpos'
	 to ptrdiff_t.

  --- src/xdisp.c 2012-06-28 12:29:37 +0000
  +++ src/xdisp.c 2012-06-29 11:48:08 +0000
  @@ -17761,8 +17761,8 @@ try_window_id (struct window *w)
       {
	 /* Displayed to end of window, but no line containing text was
	   displayed.  Lines were deleted at the end of the window.  */
  -      int first_vpos = WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0;
  -      int vpos = XFASTINT (w->window_end_vpos);
  +      ptrdiff_t first_vpos = WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0;
  +      ptrdiff_t vpos = XFASTINT (w->window_end_vpos);
	 struct glyph_row *current_row = current_matrix->rows + vpos;
	 struct glyph_row *desired_row = desired_matrix->rows + vpos;

ptrdiff_t misuse [was :Re: (empty)]

[Dmitry Antipov]

On 06/29/2012 05:20 PM, Eli Zaretskii wrote:

> Dmitry, could you please explain the reason(s) for the change below?
>
> Vertical positions in a window can never be large enough to justify
> using ptrdiff_t (on platforms where that is wider than a 32-bit
> 'int').  These are pixel positions on the Emacs display, so they
> cannot be too large.  The type of 'first_vpos' looks especially
> strange, since it is explicitly set to 1 at most.  I'm not aware of a
> platform where an 'int' is not wide enough for a value 1 ;-)

Argh, I misuse them against window_end_pos and window_end_bytepos,
which are positions in a buffer and so ptrdiff_t :-(.  This should
be reverted.

Dmitry

Re: ptrdiff_t misuse [was :Re: (empty)]

[Stefan Monnier]

> Argh, I misuse them against window_end_pos and window_end_bytepos,
> which are positions in a buffer and so ptrdiff_t :-(.  This should
> be reverted.

Not only there: hscroll and min_hscroll should be `int' fields.
And similarly Paul's "fix" to use:

  ptrdiff_t clipped_arg
    = clip_to_bounds (- w->hscroll, requested_arg, HSCROLL_MAX - w->hscroll);

should be reverted.  Really, if someone ever bumps into a problem
because of such an overflow, I'll be *super* happy, because it means
that all the performance problems we get with long lines have
been fixed.

Properly catching/handling integer overflows is a good idea, but C makes
it much too painful in general, so we don't want to do it in
unrealistic corner cases, preferring code cleanliness.



        Stefan

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

> Date: Fri, 29 Jun 2012 18:18:59 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> Argh, I misuse them against window_end_pos and window_end_bytepos,
> which are positions in a buffer and so ptrdiff_t :-(.  This should
> be reverted.

Done.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

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

On 06/29/2012 12:07 PM, Stefan Monnier wrote:
> if someone ever bumps into a problem
> because of such an overflow, I'll be*super*  happy, because it means
> that all the performance problems we get with long lines have
> been fixed.

No, if the overflow happens to generate a small integer,
Emacs will run quickly and incorrectly
without the patch in question.  Perhaps
there's a better fix, and certainly that area
of the code needs changing if hscroll
is changed to 'int' as suggested; but some fix
is needed.


[-- Attachment #2: Type: text/html, Size: 1027 bytes --]

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

> Date: Sat, 30 Jun 2012 02:13:36 -0500
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>, emacs-devel@gnu.org
> 
> if the overflow happens to generate a small integer, Emacs will run
> quickly and incorrectly without the patch in question.

But since this will _never_ happen in real life, the patch in question
is a net loss, because it obfuscates the code for no good reason.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Stefan Monnier]

>> if someone ever bumps into a problem
>> because of such an overflow, I'll be*super*  happy, because it means
>> that all the performance problems we get with long lines have
>> been fixed.
> No, if the overflow happens to generate a small integer,
> Emacs will run quickly and incorrectly
> without the patch in question.

But "fixing" the code like you have will just make it run "correctly"
and so super-extra slow that the user will just think it's frozen.
So, better keep the code clean since in any case it won't work,


        Stefan

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 06/30/2012 02:27 AM, Eli Zaretskii wrote:
> But since this will _never_ happen in real life

Surely this overflow can happen in real life, by putting the
proper strings into a text file that someone visits, which is
the sort of thing that a trickster would do.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 06/30/2012 07:07 AM, Stefan Monnier wrote:
> But "fixing" the code like you have will just make it run "correctly"
> and so super-extra slow that the user will just think it's frozen.

If Emacs simply cannot handle large hscroll values, how
about if we ceiling them out at some suitably-large value,
say 1000?  That would make this discussion moot and would
make Emacs more robust in practice, as it'd prevent some
freezes.  Emacs already limits tab width in this way, so
there's precedent.  I can work on writing up a patch along
these lines if you like.

If we can't impose an arbitrary limit, then here are some
more thoughts about the point at issue.

Although it's true that in some cases Emacs happened to run
incorrectly and quickly without the fix, in other cases
Emacs ran incorrectly and froze whereas it now runs
correctly and quickly (e.g., if the new hscroll is
sufficiently negative).  Since the patch fixes this bug
without introducing new bugs it seems like a win.

Besides, the previous paragraph assumes that signed integer
overflow silently wraps around.  This was formerly true of
Emacs porting targets but is no longer true: overflow has
undefined behavior, and can dump core or worse.  Maybe the
unfixed code happens to wrap around or dump core on all our
porting targets when overflow occurs (which appears to be
"acceptable" behavior here), but maybe it has even worse
behavior, and it's not feasible to test whether it does.
The fixed code doesn't have this problem.

I sense a larger worry here that if we methodically change
Emacs to prevent all cases of undefined behavior triggerable
by converting Emacs fixnums to C integer values, then we'll
have to add lots of checks that contort the C code so much
as to make Emacs unmaintainable.  If that's the worry, then
there should be no need for concern, as no rewrite should be
needed: the Emacs source code already has the desired
property, modulo whatever minor bugs in this area still need
fixing.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Stefan Monnier]

> If Emacs simply cannot handle large hscroll values, how

No, the problem is long lines.


        Stefan

Re: ptrdiff_t misuse [was :Re: (empty)]

[Stefan Monnier]

>> But since this will _never_ happen in real life
> Surely this overflow can happen in real life, by putting the
> proper strings into a text file that someone visits, which is
> the sort of thing that a trickster would do.

If it just causes a minor misbehavior, I'm not worried,


        Stefan

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 06/30/2012 07:23 AM, Stefan Monnier wrote:
> No, the problem is long lines.

There's also a problem even if lines are short.
I ran Emacs without the patch, on files with
only short lines, and attempted large horizontal
scroll values (say, most-positive-fixnum minus 100).
The screen would mess up.  Sometimes even after I
scrolled back to the left margin, there would be scroll
indicators in the left column.  Sometimes there would
be stray characters on the screen.

To work around this I pushed a patch (trunk bzr 108856)
that arbitrarily ceilings the hscroll value at 100000.
I didn't observe any problems with hscroll values less
than that, though I suppose the exact safe region may
vary depending on font sizes or whatnot.

This patch removes the PTRDIFF_MAX checks that
prompted this thread, as they're no longer needed.

This patch is independent of whether hscroll values
are stored as ptrdiff_t or EMACS_INT or int, so it
shouldn't affect Dmitry's work-in-progress that
would change the number of bits in an hscroll value.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

> Date: Tue, 03 Jul 2012 23:25:26 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Dmitry Antipov <dmantipov@yandex.ru>, emacs-devel@gnu.org
> 
> On 06/30/2012 07:23 AM, Stefan Monnier wrote:
> > No, the problem is long lines.
> 
> There's also a problem even if lines are short.
> I ran Emacs without the patch, on files with
> only short lines, and attempted large horizontal
> scroll values (say, most-positive-fixnum minus 100).
> The screen would mess up.  Sometimes even after I
> scrolled back to the left margin, there would be scroll
> indicators in the left column.  Sometimes there would
> be stray characters on the screen.
> 
> To work around this I pushed a patch (trunk bzr 108856)
> that arbitrarily ceilings the hscroll value at 100000.

I'm quite sure this is the wrong fix, it just sweeps the problem under
the carpet.  My guess is that, when hscroll is close to INT_MAX, the
calculations in display_line, e.g. here:

  /* Move over display elements that are not visible because we are
     hscrolled.  This may stop at an x-position < IT->first_visible_x
     if the first glyph is partially visible or if we hit a line end.  */
  if (it->current_x < it->first_visible_x)
    {
      this_line_min_pos = row->start.pos;
      move_it_in_display_line_to (it, ZV, it->first_visible_x,
				  MOVE_TO_POS | MOVE_TO_X);

and in move_it_in_display_line_to this calls, overflow, because these
calculations are done in pixels, not in character cells, and so
typically (for the default font) manipulate values that are 10 times
larger than the hscroll value.

So to fix this in the right place, we need to limit the pixel
coordinates, not the character cell coordinates.

Feel free to fix the problem that way, or leave it to me.  But
revision 108856 should be reverted.

Thanks.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 07/04/2012 09:39 AM, Eli Zaretskii wrote:
> Feel free to fix the problem that way, or leave it to me.  But
> revision 108856 should be reverted.

OK, thanks, I reverted the arbitrary imposition of a limit of 100000
in the trunk, and filed Bug#11857 about the underlying problem.
The underlying problem is not new -- I reproduced it in Emacs 23.3 --
so at least this is not a regression.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

> Date: Wed, 04 Jul 2012 11:19:12 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: monnier@iro.umontreal.ca, dmantipov@yandex.ru, emacs-devel@gnu.org
> 
> On 07/04/2012 09:39 AM, Eli Zaretskii wrote:
> > Feel free to fix the problem that way, or leave it to me.  But
> > revision 108856 should be reverted.
> 
> OK, thanks, I reverted the arbitrary imposition of a limit of 100000
> in the trunk, and filed Bug#11857 about the underlying problem.

Thanks, should be fixed now.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

Btw, what's the theory behind using ptrdiff_t in preference to
EMACS_INT to store values that came from Lisp?  E.g., in this snippet:

  static Lisp_Object
  set_window_hscroll (struct window *w, EMACS_INT hscroll)
  {
    ...
    ptrdiff_t hscroll_max = min (MOST_POSITIVE_FIXNUM, PTRDIFF_MAX);
    ptrdiff_t new_hscroll = clip_to_bounds (0, hscroll, hscroll_max);

Won't this do The Wrong Thing when Emacs is configured with wide ints?
It looks like EMACS_INT is always preferable in these contexts, as it
always has the right width.

Likewise for variables that store buffer or string positions.

What am I missing?

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 07/05/2012 12:34 PM, Eli Zaretskii wrote:
>     ptrdiff_t hscroll_max = min (MOST_POSITIVE_FIXNUM, PTRDIFF_MAX);
>     ptrdiff_t new_hscroll = clip_to_bounds (0, hscroll, hscroll_max);
> 
> Won't this do The Wrong Thing when Emacs is configured with wide ints?

No, because hscroll_max and new_hscroll cannot
exceed PTRDIFF_MAX (the above code guarantees
this directly), so their values therefore always fit
in ptrdiff_t range even when EMACS_INT is wider than
ptrdiff_t.

Similarly, buffer and string positions cannot exceed
BUF_BYTES_MAX and STRING_BYTES_MAX, which in turn
cannot exceed PTRDIFF_MAX; so ptrdiff_t always
suffices for them too.

If we change hscroll values from ptrdiff_t to int, as
I think Dmitry is proposing, the above code should be
changed to use int and INT_MAX instead of ptrdiff_t
and PTRDIFF_MAX.

Re: ptrdiff_t misuse [was :Re: (empty)]

[Eli Zaretskii]

> Date: Thu, 05 Jul 2012 17:08:50 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: dmantipov@yandex.ru, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On 07/05/2012 12:34 PM, Eli Zaretskii wrote:
> >     ptrdiff_t hscroll_max = min (MOST_POSITIVE_FIXNUM, PTRDIFF_MAX);
> >     ptrdiff_t new_hscroll = clip_to_bounds (0, hscroll, hscroll_max);
> > 
> > Won't this do The Wrong Thing when Emacs is configured with wide ints?
> 
> No, because hscroll_max and new_hscroll cannot
> exceed PTRDIFF_MAX (the above code guarantees
> this directly), so their values therefore always fit
> in ptrdiff_t range even when EMACS_INT is wider than
> ptrdiff_t.

Yes, but why do that in the first place?  The value of 'hscroll' comes
from Lisp, so it's in the range of an EMACS_INT.  Why not treat it as
such?

For that matter, why not change 'hscroll' in 'struct window' to
EMACS_INT as well?

Re: ptrdiff_t misuse [was :Re: (empty)]

[Paul Eggert]

On 07/05/2012 11:43 PM, Eli Zaretskii wrote:
> The value of 'hscroll' comes from Lisp,
> so it's in the range of an EMACS_INT.
> Why not treat it as such?
> 
> For that matter, why not change 'hscroll' in 'struct window' to
> EMACS_INT as well?

I would prefer that, yes.  Although Stefan said last week that
hscroll should be 'int'
<http://lists.gnu.org/archive/html/emacs-devel/2012-06/msg00603.html>
on the grounds that hscroll values greater than INT_MAX don't work
anyway, perhaps that's moot now that you've fixed those problems with
large hscroll values.

If we change hscroll to EMACS_INT, we should also increase the max
hscroll value to MOST_POSITIVE_FIXNUM.

Re: ptrdiff_t misuse

[Eli Zaretskii]

> Date: Fri, 06 Jul 2012 00:32:06 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: dmantipov@yandex.ru, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On 07/05/2012 11:43 PM, Eli Zaretskii wrote:
> > The value of 'hscroll' comes from Lisp,
> > so it's in the range of an EMACS_INT.
> > Why not treat it as such?
> > 
> > For that matter, why not change 'hscroll' in 'struct window' to
> > EMACS_INT as well?
> 
> I would prefer that, yes.  Although Stefan said last week that
> hscroll should be 'int'
> <http://lists.gnu.org/archive/html/emacs-devel/2012-06/msg00603.html>
> on the grounds that hscroll values greater than INT_MAX don't work
> anyway, perhaps that's moot now that you've fixed those problems with
> large hscroll values.

Stefan, do you object to making hscroll (and other values that come
from Lisp) EMACS_INT?

I think doing so will naturally and easily avoid the messy tests
against max values of several data types where we want to clip values
to some reasonable range.  So I think it will be a net win in code
clarity and maintainability, and will prevent subtle bugs.

> If we change hscroll to EMACS_INT, we should also increase the max
> hscroll value to MOST_POSITIVE_FIXNUM.

It's already that:

     ptrdiff_t hscroll_max = min (MOST_POSITIVE_FIXNUM, PTRDIFF_MAX);
     ptrdiff_t new_hscroll = clip_to_bounds (0, hscroll, hscroll_max);

(Of course, if we declare these variables EMACS_INT, PTRDIFF_MAX will
not be needed anymore.)

Or did you mean something else?

Re: ptrdiff_t misuse

[Eli Zaretskii]

> Date: Fri, 06 Jul 2012 00:32:06 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: dmantipov@yandex.ru, monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> > For that matter, why not change 'hscroll' in 'struct window' to
> > EMACS_INT as well?
> 
> I would prefer that, yes.

OK, thanks.

My next issue with ptrdiff_t use is with variables that store buffer
or string positions.  They were lately converted from EMACS_INT to
ptrdiff_t.  Here're 2 examples:

  struct buffer_text
    {
      ...
      ptrdiff_t gpt;		/* Char pos of gap in buffer.  */
      ptrdiff_t z;		/* Char pos of end of buffer.  */
      ptrdiff_t gpt_byte;		/* Byte pos of gap in buffer.  */
      ptrdiff_t z_byte;		/* Byte pos of end of buffer.  */

  struct buffer
    {
      ...
      /* Char position of point in buffer.  */
      ptrdiff_t pt;

      /* Byte position of point in buffer.  */
      ptrdiff_t pt_byte;

      /* Char position of beginning of accessible range.  */
      ptrdiff_t begv;

      /* Byte position of beginning of accessible range.  */
      ptrdiff_t begv_byte;

      /* Char position of end of accessible range.  */
      ptrdiff_t zv;

      /* Byte position of end of accessible range.  */
      ptrdiff_t zv_byte;

I understand that this is done because these variables are indices
into arrays, and thus cannot hold values that the ptrdiff_t type
cannot express.  Therefore, EMACS_INT is not good here, because it can
be wider than ptrdiff_t.

If this is indeed the reason, I suggest a new typedef:

  typedef ptrdiff_t EMACS_POS;

to be used with all such variables.  I think this will contribute to
clarity of the code, since ptrdiff_t is a rarely-used type, and thus I
expect many programmers to be unfamiliar with it.  By contrast,
EMACS_POS by its very name tells what the variable can and cannot
hold.

Comments?

Re: ptrdiff_t misuse

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > My next issue with ptrdiff_t use is with variables that store buffer
 > or string positions.  They were lately converted from EMACS_INT to
 > ptrdiff_t.  Here're 2 examples:

 > I understand that this is done because these variables are indices
 > into arrays, and thus cannot hold values that the ptrdiff_t type
 > cannot express.  Therefore, EMACS_INT is not good here, because it can
 > be wider than ptrdiff_t.
 > 
 > If this is indeed the reason, I suggest a new typedef:
 > 
 >   typedef ptrdiff_t EMACS_POS;
 > 
 > to be used with all such variables.  I think this will contribute to
 > clarity of the code,

FWIW, as an occasional reader of Emacs code, I disagree.  AIUI,
EMACS_INT exists in principle to express the idea of "C implementation
of the integer type used to implement Lisp objects" (which for some
strange reason ISO/IEC JTC1/SC22/WG14 hasn't standardized, we really
should get them to fix that! ;-), and in practice because of the
horribly inconsistent treatment of integer types across platforms, and
the fact that often Emacs will prefer a wider type to a more efficient
one (in the compiler implementer's opinion).

OTOH, ptrdiff_t expresses exactly the type required by the hardware.
I don't see how you can get more clear than that.

 > since ptrdiff_t is a rarely-used type,

EMACS_INT of course is even more rarely-used. ;-)

 > and thus I expect many programmers to be unfamiliar with it.

So teach them, just as you have to teach them EMACS_INT.  The type
name is mnemonic, as are the variables it describes.  Google brings up
plenty of relevant references.  I don't see a problem.

 > By contrast, EMACS_POS by its very name tells what the variable can
 > and cannot hold.

FWIW, to me it doesn't.  It tells me what role it plays in Emacs, it
doesn't tell me anything about the actual type, or why it needs to be
different from EMACS_INT.  I think it will be very easy for people to
confound EMACS_INT with EMACS_POS, even if they're read the typedef,
because in Lisp both kinds of value are represented by the same type.
OTOH, once a hacker learns the rationale for ptrdiff_t, I think it
will be difficult to forget.

The real problem is that lazy programmers will frequently use a
generic type (such as "int") for variables, because they can't
remember the right type.  To me, "EMACS_POS" is only marginally more
memorable than "ptrdiff_t" for this purpose, and that is more than
outweighed by the precision of "ptrdiff_t" for expressing a hardware-
dependent type.

Re: ptrdiff_t misuse

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Date: Fri, 06 Jul 2012 19:19:57 +0900
> Cc: Paul Eggert <eggert@cs.ucla.edu>, dmantipov@yandex.ru,
> 	monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> ptrdiff_t expresses exactly the type required by the hardware.

For pointer arithmetics, yes.  Which is not what we normally do with
buffer and string positions: we use them as normal integral types.

Compare that with typical declarations of array indices in C programs:
you'll rarely if ever see them declared as ptrdiff_t.

>  > since ptrdiff_t is a rarely-used type,
> 
> EMACS_INT of course is even more rarely-used. ;-)

Not for someone who hacks Emacs sources.  It's like Lisp_Object: you
get used to it very quickly and use it naturally after that.

>  > and thus I expect many programmers to be unfamiliar with it.
> 
> So teach them, just as you have to teach them EMACS_INT.  The type
> name is mnemonic, as are the variables it describes.

The type name actively resists that.  The "ptr" part is one problem,
the "diff" part is another.  None of them is related to the typical
usage of the positional values.

> [EMACS_POS] doesn't tell me anything about the actual type, or why
> it needs to be different from EMACS_INT.

Why would you care?  I don't expect J.R. Hacker to care about that,
except if she is a type junkie, in which case she just has to M-. on
the type name and see the guts.  Otherwise, all she needs to remember
is that it's "the type used for Emacs buffer/string positions".

> I think it will be very easy for people to
> confound EMACS_INT with EMACS_POS, even if they're read the typedef,
> because in Lisp both kinds of value are represented by the same type.

It is easy enough to explain that EMACS_POS is used for Lisp integers
that express buffer and string positions, and EMACS_INT for any other
Lisp integer values.

> The real problem is that lazy programmers will frequently use a
> generic type (such as "int") for variables, because they can't
> remember the right type.

I expect them to see the common practice in Emacs sources and follow
suit.  If not, they will get comments on their code to that effect and
learn fast enough.

> To me, "EMACS_POS" is only marginally more memorable than
> "ptrdiff_t" for this purpose

I didn't have you (or myself) in mind when I suggested EMACS_POS ;-)
I worry about newcomers, and would like to make their learning curve
in the matter to be as un-steep as possible.

Re: ptrdiff_t misuse

[Stephen J. Turnbull]

Eli Zaretskii writes:

 > Not for someone who hacks Emacs sources.  [EMACS_INT is] like
 > Lisp_Object: you get used to it very quickly and use it naturally
 > after that.

Well, it isn't "like" Lisp_Object; it *is* Lisp_Object!  How natural
is *that*?

 > The type name actively resists that.  The "ptr" part is one problem,
 > the "diff" part is another.  None of them is related to the typical
 > usage of the positional values.

Huh?  The only way I can understand that is that people who have no
clue about programming in C are programming in C!  Don't you find that
scary?

 > I expect them to see the common practice [using EMACS_POS] in Emacs
 > sources and follow suit.  If not, they will get comments on their
 > code to that effect and learn fast enough.

Ditto ptrdiff_t.

 > I didn't have you (or myself) in mind when I suggested EMACS_POS ;-)
 > I worry about newcomers, and would like to make their learning curve
 > in the matter to be as un-steep as possible.

I worry about newcomers, and I would prefer (if I'm going to be
reviewing) that they have a clue about programming in C if they're
going to be programming in C.  C, unlike Lisp, is the antithesis of a
"safe" language.  *Especially* when using pointers (which, in case
you've forgotten ;-) is what C arrays -- including Emacs buffer and
string data -- are!

But I'm not going to be reviewing, most likely.  I just wanted to
point out that there is a school of thought opposed to yours.

Re: ptrdiff_t misuse

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Cc: eggert@cs.ucla.edu,
>     dmantipov@yandex.ru,
>     monnier@iro.umontreal.ca,
>     emacs-devel@gnu.org
> Date: Fri, 06 Jul 2012 21:02:32 +0900
> 
> Eli Zaretskii writes:
> 
>  > Not for someone who hacks Emacs sources.  [EMACS_INT is] like
>  > Lisp_Object: you get used to it very quickly and use it naturally
>  > after that.
> 
> Well, it isn't "like" Lisp_Object; it *is* Lisp_Object!

Yes, as long as you don't use USE_LISP_UNION_TYPE (or
CHECK_LISP_OBJECT_TYPE in the current development sources), in which
case it's not.

>  > The type name actively resists that.  The "ptr" part is one problem,
>  > the "diff" part is another.  None of them is related to the typical
>  > usage of the positional values.
> 
> Huh?  The only way I can understand that is that people who have no
> clue about programming in C are programming in C!

"No clue" is an exaggeration, IMO.  One can program in C without
remembering by heart all of its obscure data types.  Latest standards
add more and more of them.

> But I'm not going to be reviewing, most likely.  I just wanted to
> point out that there is a school of thought opposed to yours.

I have no doubt that it exists, just look at the Emacs sources
lately ;-)

Re: ptrdiff_t misuse

[Paul Eggert]

On 07/06/2012 01:34 AM, Eli Zaretskii wrote:
>> If we change hscroll to EMACS_INT, we should also increase the max
>> > hscroll value to MOST_POSITIVE_FIXNUM.
> It's already that:
> 
>      ptrdiff_t hscroll_max = min (MOST_POSITIVE_FIXNUM, PTRDIFF_MAX);
>      ptrdiff_t new_hscroll = clip_to_bounds (0, hscroll, hscroll_max);
> 
> (Of course, if we declare these variables EMACS_INT, PTRDIFF_MAX will
> not be needed anymore.)
> 
> Or did you mean something else?

No, that's pretty much what I meant -- get rid of the PTRDIFF_MAX there
while we're changing ptrdiff_t to EMACS_INT for hscroll values.  Some
other minor things would need changing too, but it shouldn't be a
problem.

Re: ptrdiff_t misuse

[Paul Eggert]

On 07/06/2012 04:01 AM, Eli Zaretskii wrote:
> It is easy enough to explain that EMACS_POS is used for Lisp integers
> that express buffer and string positions, and EMACS_INT for any other
> Lisp integer values.

But not every use of ptrdiff_t is for buffer and string positions.
Bitmap table sizes, for example.  So we would need three types:

  EMACS_INT for Lisp integer values
  EMACS_POS for buffer and string positions
  ptrdiff_t for other large memory-related numbers

and at that point, we'll need to teach Emacs hackers about
ptrdiff_t anyway.

An argument for EMACS_POS is that it clearly identifies an
integer as being used for a buffer or a string position, as
having some other ptrdiff_t-like use.  But in that case shouldn't
we also distinguish between buffer positions and string positions?
And distinguish between character and byte counts, in both cases?
Something like this?

  EMACS_INT for Lisp integer values
  EMACS_BUFCPOS for buffer char positions
  EMACS_BUFBPOS for buffer byte positions
  EMACS_STRCPOS for string char positions
  EMACS_STRBPOS for string byte positions
  ptrdiff_t for other large memory-related numbers

This question answers itself, I think -- the extra complexity
isn't worth it, and we should stick with EMACS_INT and ptrdiff_t.

I think most competent C programmers know ptrdiff_t well enough.
They think of it being like size_t, but signed.  Other GNU software
tends to prefer size_t, but the Emacs style is to prefer signed
integers and ptrdiff_t is the obvious alternative.  I agree 
that most C programs use 'int' for indexes, but that's because
they're small indexes.  Emacs does this too, when indexes are
known to be sufficiently small.

Re: ptrdiff_t misuse

[Eli Zaretskii]

> Date: Fri, 06 Jul 2012 07:56:48 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: "Stephen J. Turnbull" <stephen@xemacs.org>, dmantipov@yandex.ru, 
>  monnier@iro.umontreal.ca, emacs-devel@gnu.org
> 
> On 07/06/2012 04:01 AM, Eli Zaretskii wrote:
> > It is easy enough to explain that EMACS_POS is used for Lisp integers
> > that express buffer and string positions, and EMACS_INT for any other
> > Lisp integer values.
> 
> But not every use of ptrdiff_t is for buffer and string positions.
> Bitmap table sizes, for example.

Do they really need to be large?

>   EMACS_INT for Lisp integer values
>   EMACS_POS for buffer and string positions
>   ptrdiff_t for other large memory-related numbers
> 
> and at that point, we'll need to teach Emacs hackers about
> ptrdiff_t anyway.

Not if the objects that use ptrdiff_t are many.  And perhaps we don't
need ptrdiff_t at all, if the size doesn't need to be large.

> An argument for EMACS_POS is that it clearly identifies an
> integer as being used for a buffer or a string position, as
> having some other ptrdiff_t-like use.  But in that case shouldn't
> we also distinguish between buffer positions and string positions?

What for? they span the same range of values.

> And distinguish between character and byte counts, in both cases?

What for? they span the same range of values.  And the fact that it's
a byte position is usually clearly stated in the name of the variable
itself.

>   EMACS_INT for Lisp integer values
>   EMACS_BUFCPOS for buffer char positions
>   EMACS_BUFBPOS for buffer byte positions
>   EMACS_STRCPOS for string char positions
>   EMACS_STRBPOS for string byte positions
>   ptrdiff_t for other large memory-related numbers
> 
> This question answers itself, I think -- the extra complexity
> isn't worth it

Only if you take the argument towards the absurd.

Re: ptrdiff_t misuse

[Paul Eggert]

On 07/06/2012 08:43 AM, Eli Zaretskii wrote:
>> But not every use of ptrdiff_t is for buffer and string positions.
>> > Bitmap table sizes, for example.
> Do they really need to be large?

In some cases yes -- certainly in the memory allocators.
Maybe in some cases not, but nobody has taken the time
to think about it, and in the meantime ptrdiff_t is safer.
Generally speaking, if an object can grow and its size
is not limited for other reasons to INT_MAX-or-less,
code should not use 'int' to count its size.

> What for? they span the same range of values.

Yes, that's the point -- if ptrdiff_t and EMACS_POS
would span the same range of values, why bother to
distinguish the two?  Whether a value is a
buffer position is usually clearly stated in the
variable name.

Re: ptrdiff_t misuse

[Eli Zaretskii]

> Date: Fri, 06 Jul 2012 08:56:04 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: stephen@xemacs.org, dmantipov@yandex.ru, monnier@iro.umontreal.ca, 
>  emacs-devel@gnu.org
> 
> > What for? they span the same range of values.
> 
> Yes, that's the point -- if ptrdiff_t and EMACS_POS
> would span the same range of values, why bother to
> distinguish the two?

Because Emacs positions are not pointers, nor differences between
pointers.

> Whether a value is a buffer position is usually clearly stated in
> the variable name.

I refer you to the recent repeated confusions about this by Dmitry.

Re: ptrdiff_t misuse

[Stephen J. Turnbull]

Eli Zaretskii writes:
 > > From: "Stephen J. Turnbull" <stephen@xemacs.org>

 > > Huh?  The only way I can understand that is that people who have no
 > > clue about programming in C are programming in C!
 > 
 > "No clue" is an exaggeration, IMO.  One can program in C without
 > remembering by heart all of its obscure data types.

The name "ptrdiff_t" may be obscure, but the type ptrdiff_t hardly is.
It's at the heart of C programming.  I can't imagine anybody will have
trouble remembering what it means once they learn it.

Re: ptrdiff_t misuse

[Dmitry Antipov]

On 07/06/2012 08:00 PM, Eli Zaretskii wrote:

>> Yes, that's the point -- if ptrdiff_t and EMACS_POS
>> would span the same range of values, why bother to
>> distinguish the two?
>
> Because Emacs positions are not pointers, nor differences between
> pointers.

IMHO you mix the confusing type name and it's real use.

I read ptrdiff_t exactly as "difference between pointers", and this is
the minor drawback; the major one is that there is no standard unsigned
counterpart, like for size_t/ssize_t pair.

>> Whether a value is a buffer position is usually clearly stated in
>> the variable name.
>
> I refer you to the recent repeated confusions about this by Dmitry.

This isn't related to any type (mis)use. In a struct window,
window_end_pos really means something like last_displayed_pos;
this is quite hard to mix with window_end_[whatever].

Dmitry

Re: ptrdiff_t misuse

[Paul Eggert]

On 07/06/2012 09:42 AM, Dmitry Antipov wrote:
> I read ptrdiff_t exactly as "difference between pointers", and this is
> the minor drawback; the major one is that there is no standard unsigned
> counterpart, like for size_t/ssize_t pair.

For buffer positions and the like, Emacs code can take size_t to be
the unsigned counterpart of ptrdiff_t.  For Emacs we typically prefer
signed integers, so the need for size_t is rare -- in fact some of the
current uses of size_t should be changed to ptrdiff_t, for consistency
with the usual Emacs style.

There are hosts with size_t wider than ssize_t (POSIX allows this).
That is why we use ptrdiff_t, not ssize_t, for buffer positions and
the like -- on those hosts, ssize_t wouldn't work for large buffers.
Conversely, on some (rare) hosts ptrdiff_t is wider than size_t (POSIX
allows this too).  As far as I know Emacs has never been ported to
these hosts but it should work, since buffer positions will fit into
ptrdiff_t.

Re: ptrdiff_t misuse

[Stefan Monnier]

> Stefan, do you object to making hscroll (and other values that come
> from Lisp) EMACS_INT?

Yes.  A large value makes no sense.  It should be int.
And I'm perfectly happy with taking very-large values and letting them
wraparound.  Those values are bogus anyway.


        Stefan

Re: ptrdiff_t misuse

[Samuel Bronson]

On Jul 6, 2012, at 5:30 PM, Stefan Monnier wrote:

>> Stefan, do you object to making hscroll (and other values that come
>> from Lisp) EMACS_INT?
>
> Yes.  A large value makes no sense.  It should be int.
> And I'm perfectly happy with taking very-large values and letting them
> wraparound.  Those values are bogus anyway.

... but isn't that Undefined Behavior?

Re: ptrdiff_t misuse

[Chong Yidong]

Eli Zaretskii <eliz@gnu.org> writes:

> If this is indeed the reason, I suggest a new typedef:
>
>   typedef ptrdiff_t EMACS_POS;
>
> to be used with all such variables.  I think this will contribute to
> clarity of the code, since ptrdiff_t is a rarely-used type, and thus I
> expect many programmers to be unfamiliar with it.  By contrast,
> EMACS_POS by its very name tells what the variable can and cannot
> hold.

This is a bad idea.  Since EMACS_POS would occur rarely in the code, it
would be necessary to look up what it means when you stumble across it.
It would be easier to simply remember the meaning and use of ptrdiff_t.
Introducing yet another type defeats the purpose.

Re: ptrdiff_t misuse

[Stefan Monnier]

>>> Stefan, do you object to making hscroll (and other values that come
>>> from Lisp) EMACS_INT?
>> Yes.  A large value makes no sense.  It should be int.
>> And I'm perfectly happy with taking very-large values and letting them
>> wraparound.  Those values are bogus anyway.
> ... but isn't that Undefined Behavior?

In the context of Emacs I'm not scared of undefined behavior.


        Stefan

Re: ptrdiff_t misuse

[Paul Eggert]

On 07/07/2012 03:59 AM, Stefan Monnier wrote:
> In the context of Emacs I'm not scared of undefined behavior.

Emacs uses undefined behavior all the time, and there's
nothing wrong with that.  But it has to be careful about
which undefined behavior it can rely on.  In the old days,
it was fine for Emacs to assume that signed integer overflow
wraps around.  But because compilers have gotten much fancier
at optimizing, the corresponding undefined behavior no longer
results in simple wraparound, but can lead to subtle logic
bugs far away from the offending code.

Over the past couple of years we've changed Emacs so that it
is now fairly good at avoiding signed integer overflow.
One can build it with gcc -ftrapv and it doesn't trap, for example.
This kind of analysis and testing helps to make Emacs more reliable.
When it doesn't significantly impede more-important concerns
it should be encouraged, even if it isn't the highest-priority task.