Your changes in revision 106240

Your changes in revision 106240

[Eli Zaretskii]

  -which_symbols (Lisp_Object obj, int find_max)
  +which_symbols (Lisp_Object obj, EMACS_INT find_max)

Why? what possible harm can be done by using an int here?  And what
does EMACS_INT have to do with the maximum number of symbols that the
caller wants to see?

Re: Your changes in revision 106240

[Paul Eggert]

On 10/29/11 13:54, Eli Zaretskii wrote:
> what possible harm can be done by using an int here?

On a typical 64-bit host, using 'int' breaks a GDB command like
"xwhichsymbols Qnil 4294967297", by causing xwhichsymbols
to silently treat the 4294967297 as if it were 1.

For longstanding Emacs code this kind of fix is being put off
until after the release (see bug 9874) but newly-added
code like which_symbols should avoid arbitrary limits
unless there's a good reason to have them.

Re: Your changes in revision 106240

[Stefan Monnier]

>> what possible harm can be done by using an int here?
> On a typical 64-bit host, using 'int' breaks a GDB command like
> "xwhichsymbols Qnil 4294967297", by causing xwhichsymbols
> to silently treat the 4294967297 as if it were 1.

But this has nothing to do with EMACS_INT.  It's at best a misfeature in
the GDB->C interfacing that doesn't warn of such rounding, tho one might
argue that they are intentional and that a programmer should expect them.

In any case, this arg is *never* going to be that large.


        Stefan

Re: Your changes in revision 106240

[Paul Eggert]

On 10/29/11 22:06, Stefan Monnier wrote:
> But this has nothing to do with EMACS_INT.

Actually, there's a direct connection to EMACS_INT.
That integer gives an upper bound on the length of the list FOUND.
Emacs list lengths fit into EMACS_INT, not int,
and variables that contain Emacs list lengths
should therefore be of type EMACS_INT, not int.

> this arg is *never* going to be that large

It was in my example :-).  In any case, the fix is trivial and safe.

Re: Your changes in revision 106240

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Sun, 30 Oct 2011 01:06:16 -0400
> 
> >> what possible harm can be done by using an int here?
> > On a typical 64-bit host, using 'int' breaks a GDB command like
> > "xwhichsymbols Qnil 4294967297", by causing xwhichsymbols
> > to silently treat the 4294967297 as if it were 1.
> 
> But this has nothing to do with EMACS_INT.  It's at best a misfeature in
> the GDB->C interfacing that doesn't warn of such rounding, tho one might
> argue that they are intentional and that a programmer should expect them.

Even if we wanted to support such ridiculously large values, TRT would
be to make that argument `long', not EMACS_INT, because we have the
"--with-wide-int" configuration where EMACS_INT is a 64-bit type
whereas GDB will pass a 32-bit value.

> In any case, this arg is *never* going to be that large.

Right.  And moreover, a GDB user who stupidly passes such a large
value will hurt no one but herself, so we don't need to protect Emacs
from that.

Re: Your changes in revision 106240

[Eli Zaretskii]

> Date: Sat, 29 Oct 2011 23:06:38 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> Actually, there's a direct connection to EMACS_INT.
> That integer gives an upper bound on the length of the list FOUND.

And using `int' makes sure we never exceed that upper bound.

Re: Your changes in revision 106240

[Stefan Monnier]

>> But this has nothing to do with EMACS_INT.
> Actually, there's a direct connection to EMACS_INT.

I don't see it.

> That integer gives an upper bound on the length of the list FOUND.

Right.

> Emacs list lengths fit into EMACS_INT, not int,

That's not relevant: this argument is not bound to match the length of
some arbitrary list, instead it specifies the maximum size of the list
to build.  So it could be `char' or `short' and that would work just as well.


        Stefan

Re: Your changes in revision 106240

[Paul Eggert]

On 10/29/11 23:16, Eli Zaretskii wrote:

> "--with-wide-int" configuration where EMACS_INT is a 64-bit type
> whereas GDB will pass a 32-bit value.

No, GDB passes 64-bit values to functions that are compiled to
accept 64-bit values.  There is no unwanted truncation.  It works
fine, even on 32-bit hosts configured --with-wide-int.

Re: Your changes in revision 106240

[Paul Eggert]

On 10/29/11 23:31, Stefan Monnier wrote:
> it could be `char' or `short' and that would work just as well

But by that line of reasoning, the parameter could be 'bool', which
would also work just as well, so long as one likes the arbitrary
limit of 1 that is imposed by 'bool'.

We should avoid arbitrary limits on the lengths of data structures.
Limiting that list length to 1, or to 255, or to 65535, would impose
an arbitrary limit.

Re: Your changes in revision 106240

[Eli Zaretskii]

> Date: Sun, 30 Oct 2011 00:15:34 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> Cc: emacs-devel@gnu.org
> 
> We should avoid arbitrary limits on the lengths of data structures.

It's not arbitrary.  It is unreasonable to ask GDB to display a list
that is longer than a small number of items.

Re: Your changes in revision 106240

[Stefan Monnier]

> We should avoid arbitrary limits on the lengths of data structures.

We're not bound by such a dogma.  While it's generally true that we try
to do that, we don't do it at any cost in every single possible case.


        Stefan

Re: Your changes in revision 106240

[Paul Eggert]

On 10/30/11 00:27, Eli Zaretskii wrote:
> It is unreasonable to ask GDB to display a list
> that is longer than a small number of items.

First, list display need not be involved.  For example:

  set $len = Flength (which_symbols (Qnil, $my_limit))
  pp $len

These GDB commands print a count of how many symbols have
the value nil, while not bothering to count any symbols more than
an upper bound of $my_limit.  On a 64-bit host, there's no good
reason for these commands to misbehave merely because $my_limit
exceeds 2**32.

Second, even if display is involved, it is sometimes reasonable to
ask for the display of a long list.  For example, when debugging
Emacs under GDB, I sometimes generate long outputs which I then
analyze with other tools.

Re: Your changes in revision 106240

[Paul Eggert]

On 10/30/11 01:07, Stefan Monnier wrote:
> While it's generally true that we try to do that,
> we don't do it at any cost in every single possible case.

Yes, avoiding arbitrary limits is a guideline not an absolute
goal, and other goals can override it.  But in the case we're
talking about, avoiding the 32-bit limit is trivial and costs
nothing, and cost concerns do not support overriding the guideline
in this case.

That being said, the length of this discussion suggests that I may
have touched a nerve, which was not my intent.  I'd rather that we
didn't get bogged down on this during the pretest.  How about the
following idea for moving things forward?  I'll stop fixing
newly-introduced integer-width issues in the trunk, and instead will
fold any such issues into the patch for bug 9874, a patch that
won't be applied until after the next release.  We can discuss
matters further then as needed.