Allow value of PRINT_CIRCLE to be modified from Elisp?

Allow value of PRINT_CIRCLE to be modified from Elisp?

[Toby Cubitt]

print.c defines an arbitrary fixed limit

  #define PRINT_CIRCLE 200

on the depth to which a lisp object can be printed, before it bails out
with the "Apparently circular structure being printed" error.

Apparently, no one anticipated wanting to print highly nested Elisp
structures. But this arbitrary limit is breaking some of the
functionality in Elisp packages I maintain (see below for a detailed
explanation).

Could the #define PRINT_CIRCLE constant be turned into a DEFVAR_INT
`max-lisp-print-depth' variable, so that it could be adjusted from Elisp
when more print depth is needed?


The motivation for this request is that I've been playing around with
implementing persistent storage of undo history in undo-tree.el. This is
trivial to implement these days thanks to recent Emacs' ability to `read'
back the output of non-nil print-circle `prin1' output.

Unfortunately, undo history typically grows very long, even in a
moderately heavily edited buffer, creating very deep undo-trees. For all
but the most trivial test buffers, saving undo-tree history to file using
prin1 hits the arbitrary PRINT_CIRCLE limit and fails.

I've hit exactly the same problem before in my predictive.el package
(actually, in dict-tree.el, which implements the trie-based dictionary
data structures I use there). If one stores long strings in the
dictionaries, persistent storage of the dictionaries (again just
implemented using the natural prin1 method) hits PRINT_CIRCLE and fails.

So I now have two real-world use cases where the PRINT_CIRCLE limit is
causing problems.

I know I could work-around the problem using a more complicated
persistent storage implementation than prin1. But this would effectively
mean writing my own data-structure-specific re-implementations of prin1.
And that just seems wrong when we have all the beauty of Lisp read/print
syntax to hand.


Because the depth of the dictionary trees is proportional to the longest
string they store, which is typically upper-bounded by some not-too-big
constant, it's quite rare to hit this issue at all in predictive.el. And
the simple solution of increasing the value of PRINT_CIRCLE in print.c
and recompiling avoids the problem in any real-world use. (Though the
average Emacs user probably wouldn't call this solution "simple"!)

But the depth of an undo-tree is proportional to the length of the undo
history. PRINT_CIRCLE would have to be very large to avoid the problem in
undo-tree.el, probably too big to usefully protect against infinite
recursion elsewhere. An Elisp-configurable limit would seem the better
way to go, so that it can temporarily be let-bound to a larger value when
printing the undo-trees.

eval.c has adjustable `max-lisp-eval-depth', with big scary warnings in
the docstring about potential stack overflows if it's increased too
much. Could we add a `max-lisp-print-depth', with similar warnings?


I looked into writing a patch for this myself, but print.c defines
being_printed as a global static array for use in print_preprocess and
print_object:

  static Lisp_Object being_printed[PRINT_CIRCLE];

This would have to be turned into a dynamically allocated array
(allocated in the print c function?), and I'm not familiar enough with
Emacs' c code memory management to know how to implement this correctly.

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Stefan Monnier]

> print.c defines an arbitrary fixed limit
>   #define PRINT_CIRCLE 200

> on the depth to which a lisp object can be printed, before it bails out
> with the "Apparently circular structure being printed" error.

> Apparently, no one anticipated wanting to print highly nested Elisp
> structures. But this arbitrary limit is breaking some of the
> functionality in Elisp packages I maintain (see below for a detailed
> explanation).

> Could the #define PRINT_CIRCLE constant be turned into a DEFVAR_INT
> `max-lisp-print-depth' variable, so that it could be adjusted from Elisp
> when more print depth is needed?

Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.


        Stefan

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Toby Cubitt]

On Thu, Apr 19, 2012 at 06:12:15PM -0400, Stefan Monnier wrote:
> > print.c defines an arbitrary fixed limit
> >   #define PRINT_CIRCLE 200
> 
> > on the depth to which a lisp object can be printed, before it bails out
> > with the "Apparently circular structure being printed" error.
> 
> > Apparently, no one anticipated wanting to print highly nested Elisp
> > structures. But this arbitrary limit is breaking some of the
> > functionality in Elisp packages I maintain (see below for a detailed
> > explanation).
> 
> > Could the #define PRINT_CIRCLE constant be turned into a DEFVAR_INT
> > `max-lisp-print-depth' variable, so that it could be adjusted from Elisp
> > when more print depth is needed?
> 
> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.

That would definitely solve all my problems if it can be done.

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Andreas Schwab]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.

You still need to dynamically expand being_printed.

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: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Stefan Monnier]

>> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.
> You still need to dynamically expand being_printed.

AFAICT being_printed is only used when print-circle is nil.


        Stefan

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Andreas Schwab]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>>> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.
>> You still need to dynamically expand being_printed.
>
> AFAICT being_printed is only used when print-circle is nil.

You're right, I misread the conditions.

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: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Stefan Monnier]

>> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.
> That would definitely solve all my problems if it can be done.

Done in the trunk,


        Stefan

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Toby Cubitt]

On Fri, Apr 20, 2012 at 09:03:14AM -0400, Stefan Monnier wrote:
> >> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is non-nil.
> > That would definitely solve all my problems if it can be done.
> 
> Done in the trunk,

Thanks! I've been testing persistent undo-tree history since this change
was applied, and it seems to be working fine now.

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Toby Cubitt]

On Sat, Apr 21, 2012 at 01:44:23PM +0200, Toby Cubitt wrote:
> On Fri, Apr 20, 2012 at 09:03:14AM -0400, Stefan Monnier wrote:
> > >> Maybe we should simply ignore PRINT_CIRCLE when `print-circle' is
> > >> non-nil. 
> > >
> > > That would definitely solve all my problems if it can be done.
> > 
> > Done in the trunk,
> 
> Thanks! I've been testing persistent undo-tree history since this change
> was applied, and it seems to be working fine now.

It seems this fix didn't make it into the emacs-24 branch. Any reason?

Here's the commit:

------------------------------------------------------------
revno: 107975
committer: Stefan Monnier <monnier@iro.umontreal.ca>
branch nick: trunk
timestamp: Fri 2012-04-20 09:02:20 -0400
message:
  * src/print.c (print_preprocess): Only check print_depth if print-circle
  is nil.
  (print_object): Check for cycles even when print-circle is nil and
  print-gensym is t, but only check print_depth if print-circle is nil.
------------------------------------------------------------

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Stefan Monnier]

> It seems this fix didn't make it into the emacs-24 branch. Any reason?

Mostly an accident, although I also wasn't completely sure it was
a safe change.
In any case, the emacs-24 branch should receive all of `trunk' in
a month or so (we just froze the trunk), so it's a bit late to fix it.


        Stefan

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Toby Cubitt]

On Tue, Oct 02, 2012 at 09:23:50AM -0400, Stefan Monnier wrote:
> > It seems this fix didn't make it into the emacs-24 branch. Any reason?
> 
> Mostly an accident, although I also wasn't completely sure it was
> a safe change.
> In any case, the emacs-24 branch should receive all of `trunk' in
> a month or so (we just froze the trunk), so it's a bit late to fix it.

Yup, I was aware of the feature freeze (but unfortunately didn't realise
the fix hadn't made the emacs-24 branch until today, when Donald pointed
it out).

Does this mean this fix won't make the 24.3 release, but will (probably)
make the 24.4 release?

If that's correct, I'll update the version number in the warning I added
to undo-tree (to warn people that persistent undo storage won't work in
Emacs versions lacking this fix).

Thanks,

Toby
-- 
Dr T. S. Cubitt
Mathematics and Quantum Information group
Department of Mathematics
Complutense University
Madrid, Spain

email: tsc25@cantab.net
web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Donald Ephraim Curtis]

I think it means it *will* make the 24.3 release. My understanding is that what is currently in `trunk` will become 24.3.

dc


On Oct 2, 2012, at 09:04, Toby Cubitt <tsc25@cantab.net> wrote:

> On Tue, Oct 02, 2012 at 09:23:50AM -0400, Stefan Monnier wrote:
>>> It seems this fix didn't make it into the emacs-24 branch. Any reason?
>> 
>> Mostly an accident, although I also wasn't completely sure it was
>> a safe change.
>> In any case, the emacs-24 branch should receive all of `trunk' in
>> a month or so (we just froze the trunk), so it's a bit late to fix it.
> 
> Yup, I was aware of the feature freeze (but unfortunately didn't realise
> the fix hadn't made the emacs-24 branch until today, when Donald pointed
> it out).
> 
> Does this mean this fix won't make the 24.3 release, but will (probably)
> make the 24.4 release?
> 
> If that's correct, I'll update the version number in the warning I added
> to undo-tree (to warn people that persistent undo storage won't work in
> Emacs versions lacking this fix).
> 
> Thanks,
> 
> Toby
> -- 
> Dr T. S. Cubitt
> Mathematics and Quantum Information group
> Department of Mathematics
> Complutense University
> Madrid, Spain
> 
> email: tsc25@cantab.net
> web:   www.dr-qubit.org

Re: Allow value of PRINT_CIRCLE to be modified from Elisp?

[Stefan Monnier]

> I think it means it *will* make the 24.3 release. My understanding is that
> what is currently in `trunk` will become 24.3.

Exactly.


        Stefan