unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
* port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
@ 2014-05-17 18:10 Doug Evans
  2014-05-21 14:24 ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Doug Evans @ 2014-05-17 18:10 UTC (permalink / raw)
  To: guile-devel

Hi.

I've been playing with gdb objects implemented as structs and have hit
an oddity.

The problem can be succinctly represented by the following:

scheme@(guile-user)> (port? (port-with-print-state (current-output-port)))
$3 = #f

I've dug into the implementation of the functions and macros involved.
I think I understand why it's printing #f.
A port-with-print-state is a scm_tc16_port_with_ps port, not a
scm_tc7_port port.

This means that any function that accepts a "port" parameter and
wishes to call functions like scm_puts has to handle this distinction
(because scm_puts only accepts a scm_tc7_port port).

Has anyone done an audit of the C API with an eye towards making any
function that claims to take a port argument accept either kind of
port?

The current situation is a bit confusing.
E.g., in the docs for scm_set_smob_print, I see this text:

It is often best to ignore @var{pstate} and just print to @var{port}
with @code{scm_display}, @code{scm_write}, @code{scm_simple_format},
and @code{scm_puts}.

scm_display and scm_write are smart enough to handle both kinds of
ports, but scm_puts is not.  I didn't check scm_simple_format.
And I don't see any docs indicating one needs to be aware of this port
vs port-with-pstate-port distinction in smob or struct printers.

Users can be pretty simplistic when reading documentation, this user
included. :-)
Words tend to take on meanings that if used in one place are expected
to apply everywhere.
IOW, a port is a port is a port.
In that vein, should port? [et.al.] return #t for a
port-with-print-state port?  Dunno.

I dug around looking for various problems in the guile sources that
might be caused by this confusion.  I couldn't find any and I think
it's because struct printing is the only place that uses
scm_printer_apply.  Every other object printer I found takes port and
pstate as separate arguments and scm_prin1 handles splitting out
pstate from the incoming port parameter, thus all printers will only
ever see a scm_tc7_port, never a scm_tc16_port_with_ps port (and thus
all printers that call scm_puts never trip over this confusion, only
struct printers).

In the meantime, I can make struct printers be aware of the
distinction and handle being passed port-with-print-state ports.

P.S. How come scm_put[cs]_unlocked are inlined in ports.h?



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-05-17 18:10 port-with-print-state doesn't create a port? Or, when is a port not a port? :-) Doug Evans
@ 2014-05-21 14:24 ` Ludovic Courtès
  2014-05-25 19:05   ` Doug Evans
  2014-05-26 14:47   ` Mark H Weaver
  0 siblings, 2 replies; 7+ messages in thread
From: Ludovic Courtès @ 2014-05-21 14:24 UTC (permalink / raw)
  To: guile-devel

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

Hi!

Doug Evans <xdje42@gmail.com> skribis:

> The problem can be succinctly represented by the following:
>
> scheme@(guile-user)> (port? (port-with-print-state (current-output-port)))
> $3 = #f

I think the short answer is that it’s a very old API that’s essentially
unused internally.  For instance, make check passes with this patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 1722 bytes --]

index 122e035..8f4c969 100644
--- a/libguile/print.c
+++ b/libguile/print.c
@@ -255,6 +255,7 @@ scm_free_print_state (SCM print_state)
 SCM
 scm_i_port_with_print_state (SCM port, SCM print_state)
 {
+  abort ();
   if (SCM_UNBNDP (print_state))
     {
       if (SCM_PORT_WITH_PS_P (port))
@@ -596,8 +597,7 @@ iprin1 (SCM exp, SCM port, scm_print_state *pstate)
 		SCM pwps, print = pstate->writingp ? g_write : g_display;
 		if (SCM_UNPACK (print) == 0)
 		  goto print_struct;
-		pwps = scm_i_port_with_print_state (port, pstate->handle);
-		pstate->revealed = 1;
+		pwps = port;
 		scm_call_generic_2 (print, exp, pwps);
 	      }
 	    else
@@ -824,6 +824,7 @@ scm_prin1 (SCM exp, SCM port, int writingp)
 
   if (SCM_PORT_WITH_PS_P (port))
     {
+      abort ();
       pstate_scm = SCM_PORT_WITH_PS_PS (port);
       port = SCM_PORT_WITH_PS_PORT (port);
     }
@@ -1610,7 +1611,7 @@ scm_printer_apply (SCM proc, SCM exp, SCM port, scm_print_state *pstate)
 {
   pstate->revealed = 1;
   return scm_call_2 (proc, exp,
-		     scm_i_port_with_print_state (port, pstate->handle));
+		     port);
 }
 
 SCM_DEFINE (scm_port_with_print_state, "port-with-print-state", 1, 1, 0, 
diff --git a/module/ice-9/boot-9.scm b/module/ice-9/boot-9.scm
index 42d7d78..2974ef6 100644
--- a/module/ice-9/boot-9.scm
+++ b/module/ice-9/boot-9.scm
@@ -1257,11 +1257,6 @@ VALUE."
 ;;
 ;; It should print OBJECT to PORT.
 
-(define (inherit-print-state old-port new-port)
-  (if (get-print-state old-port)
-      (port-with-print-state new-port (get-print-state old-port))
-      new-port))
-
 ;; 0: type-name, 1: fields, 2: constructor
 (define record-type-vtable
   (let ((s (make-vtable (string-append standard-vtable-fields "prprpw")

[-- Attachment #3: Type: text/plain, Size: 698 bytes --]


I think the problem it was trying to solve has been solved differently
(by explicitly passing the print state in the print.c code, for
instance), and can easily be solved differently.

> In the meantime, I can make struct printers be aware of the
> distinction and handle being passed port-with-print-state ports.

Do you actually need this associated state?

My first feeling is that we should deprecate and eventually remove this
API, given that it’s essentially unused and non-functional.

What do people think?

> P.S. How come scm_put[cs]_unlocked are inlined in ports.h?

What’s wrong with that?  (It probably doesn’t help much because
scm_lfwrite_unlocked isn’t inlined.)

Ludo’.

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-05-21 14:24 ` Ludovic Courtès
@ 2014-05-25 19:05   ` Doug Evans
  2014-05-26 14:47   ` Mark H Weaver
  1 sibling, 0 replies; 7+ messages in thread
From: Doug Evans @ 2014-05-25 19:05 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

On Wed, May 21, 2014 at 7:24 AM, Ludovic Courtès <ludo@gnu.org> wrote:
> Hi!
>
> Doug Evans <xdje42@gmail.com> skribis:
>
>> The problem can be succinctly represented by the following:
>>
>> scheme@(guile-user)> (port? (port-with-print-state (current-output-port)))
>> $3 = #f
>
> I think the short answer is that it’s a very old API that’s essentially
> unused internally.  For instance, make check passes with this patch:
>
>
>
> I think the problem it was trying to solve has been solved differently
> (by explicitly passing the print state in the print.c code, for
> instance), and can easily be solved differently.
>
>> In the meantime, I can make struct printers be aware of the
>> distinction and handle being passed port-with-print-state ports.
>
> Do you actually need this associated state?

Nope.

The point of my message is that if a struct printer written in C
blindly calls scm_puts ... boom.
That, plus it's a bit odd that port-with-print-state doesn't create
something that satisfies port?.

> My first feeling is that we should deprecate and eventually remove this
> API, given that it’s essentially unused and non-functional.
>
> What do people think?
>
>> P.S. How come scm_put[cs]_unlocked are inlined in ports.h?
>
> What’s wrong with that?  (It probably doesn’t help much because
> scm_lfwrite_unlocked isn’t inlined.)

AFAICT there can be no measurable speed up from having them there, so
their presence is at least odd: The astute reader of the file will ask
why they're there, and there is no good answer.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-05-21 14:24 ` Ludovic Courtès
  2014-05-25 19:05   ` Doug Evans
@ 2014-05-26 14:47   ` Mark H Weaver
  2014-05-26 16:26     ` Ludovic Courtès
  1 sibling, 1 reply; 7+ messages in thread
From: Mark H Weaver @ 2014-05-26 14:47 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

ludo@gnu.org (Ludovic Courtès) writes:

> Doug Evans <xdje42@gmail.com> skribis:
>
>> The problem can be succinctly represented by the following:
>>
>> scheme@(guile-user)> (port? (port-with-print-state (current-output-port)))
>> $3 = #f
>
> I think the short answer is that it’s a very old API that’s essentially
> unused internally.  [...]
[...]
> I think the problem it was trying to solve has been solved differently
> (by explicitly passing the print state in the print.c code, for
> instance), and can easily be solved differently.

In order to implement SRFI-38 properly and efficiently, I think we need
to somehow pass the print state to user-defined structure printers.
Among other things, the print state includes a map from the set of
objects that are currently being printed (i.e. the ancestors of the
current object) to the associated datum label.

Aside from proper SRFI-38 support, the print state is also used to
specify parameters such as maximum-depth for printing abbreviated
structures, used for example by the backtrace printer.

As distasteful as this 'port-with-print-state' concept may be, I'm not
aware of a better solution.  Fluids aren't quite right, because a
structure printer might cause I/O to happen on another port.

Another alternative would be to explicitly pass the print state to
structure printers, and then provide versions of 'write' and 'display'
that accept a separate print state argument, but we'd still need to
handle all the existing struct printers that don't know about this.

Yet another option would be to move the print state into the port
itself.  It might be worth considering, although it seems a bit unclean.

  Thoughts?
     Mark



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-05-26 14:47   ` Mark H Weaver
@ 2014-05-26 16:26     ` Ludovic Courtès
  2014-06-03 22:57       ` Mark H Weaver
  0 siblings, 1 reply; 7+ messages in thread
From: Ludovic Courtès @ 2014-05-26 16:26 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guile-devel

Mark H Weaver <mhw@netris.org> skribis:

> ludo@gnu.org (Ludovic Courtès) writes:
>
>> Doug Evans <xdje42@gmail.com> skribis:
>>
>>> The problem can be succinctly represented by the following:
>>>
>>> scheme@(guile-user)> (port? (port-with-print-state (current-output-port)))
>>> $3 = #f
>>
>> I think the short answer is that it’s a very old API that’s essentially
>> unused internally.  [...]
> [...]
>> I think the problem it was trying to solve has been solved differently
>> (by explicitly passing the print state in the print.c code, for
>> instance), and can easily be solved differently.
>
> In order to implement SRFI-38 properly and efficiently, I think we need
> to somehow pass the print state to user-defined structure printers.
> Among other things, the print state includes a map from the set of
> objects that are currently being printed (i.e. the ancestors of the
> current object) to the associated datum label.
>
> Aside from proper SRFI-38 support, the print state is also used to
> specify parameters such as maximum-depth for printing abbreviated
> structures, used for example by the backtrace printer.

Good points.

> As distasteful as this 'port-with-print-state' concept may be, I'm not
> aware of a better solution.  Fluids aren't quite right, because a
> structure printer might cause I/O to happen on another port.
>
> Another alternative would be to explicitly pass the print state to
> structure printers, and then provide versions of 'write' and 'display'
> that accept a separate print state argument, but we'd still need to
> handle all the existing struct printers that don't know about this.
>
> Yet another option would be to move the print state into the port
> itself.  It might be worth considering, although it seems a bit unclean.

Maybe the port alist you added a few months ago could be used to
implement that actually, no?

Thanks,
Ludo’.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-05-26 16:26     ` Ludovic Courtès
@ 2014-06-03 22:57       ` Mark H Weaver
  2014-06-04  8:25         ` Ludovic Courtès
  0 siblings, 1 reply; 7+ messages in thread
From: Mark H Weaver @ 2014-06-03 22:57 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guile-devel

I wrote earlier:

> As distasteful as this 'port-with-print-state' concept may be, I'm not
> aware of a better solution.  Fluids aren't quite right, because a
> structure printer might cause I/O to happen on another port.

Having thought more on this, I think fluids might be the right tool.

The only detail is, the print state would have to include a reference to
the associated port.  Then, if the port passed to 'write' or 'display'
doesn't match the one associated with the current-print-state, it would
be saved and later restored, with a fresh new current-print-state used
for the duration of that 'write' or 'display' call.

What do you think?

      Mark



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: port-with-print-state doesn't create a port? Or, when is a port not a port? :-)
  2014-06-03 22:57       ` Mark H Weaver
@ 2014-06-04  8:25         ` Ludovic Courtès
  0 siblings, 0 replies; 7+ messages in thread
From: Ludovic Courtès @ 2014-06-04  8:25 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guile-devel

Mark H Weaver <mhw@netris.org> skribis:

> I wrote earlier:
>
>> As distasteful as this 'port-with-print-state' concept may be, I'm not
>> aware of a better solution.  Fluids aren't quite right, because a
>> structure printer might cause I/O to happen on another port.
>
> Having thought more on this, I think fluids might be the right tool.
>
> The only detail is, the print state would have to include a reference to
> the associated port.  Then, if the port passed to 'write' or 'display'
> doesn't match the one associated with the current-print-state, it would
> be saved and later restored, with a fresh new current-print-state used
> for the duration of that 'write' or 'display' call.
>
> What do you think?

Yes, it seems like it should work, and I find it natural.  We’d have to
check on a concrete use case.

Ludo’.



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2014-06-04  8:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-17 18:10 port-with-print-state doesn't create a port? Or, when is a port not a port? :-) Doug Evans
2014-05-21 14:24 ` Ludovic Courtès
2014-05-25 19:05   ` Doug Evans
2014-05-26 14:47   ` Mark H Weaver
2014-05-26 16:26     ` Ludovic Courtès
2014-06-03 22:57       ` Mark H Weaver
2014-06-04  8:25         ` Ludovic Courtès

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).