[PATCH] Source properties on arbitrary non-immediate values

[PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Hello,

Does anyone know the rationale for allowing to attach source properties
_only_ to pairs?

Since I couldn't think of any compelling reason, I tried to remove this
limitation, and still I can't see any problem.  :-)  The patch below
allows to do things like this:

  guile> (define x 'a-symbol)
  guile> (set-source-properties! x '((a . 2)))
  ((a . 2))
  guile> (source-properties x)
  ((a . 2))
  guile> (define s "a string")
  guile> (set-source-properties! s '((line . 12)))
  ((line . 12))
  guile> (source-property s 'line)
  12

Thanks,
Ludovic.


2005-10-07  Ludovic Courtès  <ludovic.courtes@laas.fr>

	* srcprop.c (source-properties):  Accept non-pair objects.
	(set-source-properties!):  Likewise.
	(source-property):  Likewise.
	(set-source-property!):  Likewise.


\f
--- orig/libguile/srcprop.c
+++ mod/libguile/srcprop.c
@@ -157,8 +157,7 @@
   SCM_VALIDATE_NIM (1, obj);
   if (SCM_MEMOIZEDP (obj))
     obj = SCM_MEMOIZED_EXP (obj);
-  else if (!scm_is_pair (obj))
-    SCM_WRONG_TYPE_ARG (1, obj);
+
   p = scm_hashq_ref (scm_source_whash, obj, SCM_EOL);
   if (SRCPROPSP (p))
     return scm_srcprops_to_plist (p);
@@ -180,8 +179,7 @@
   SCM_VALIDATE_NIM (1, obj);
   if (SCM_MEMOIZEDP (obj))
     obj = SCM_MEMOIZED_EXP (obj);
-  else if (!scm_is_pair (obj))
-    SCM_WRONG_TYPE_ARG(1, obj);
+
   handle = scm_hashq_create_handle_x (scm_source_whash, obj, plist);
   SCM_SETCDR (handle, plist);
   return plist;
@@ -198,8 +196,7 @@
   SCM_VALIDATE_NIM (1, obj);
   if (SCM_MEMOIZEDP (obj))
     obj = SCM_MEMOIZED_EXP (obj);
-  else if (!scm_is_pair (obj))
-    SCM_WRONG_TYPE_ARG (1, obj);
+
   p = scm_hashq_ref (scm_source_whash, obj, SCM_EOL);
   if (!SRCPROPSP (p))
     goto plist;
@@ -230,8 +227,7 @@
   SCM_VALIDATE_NIM (1, obj);
   if (SCM_MEMOIZEDP (obj))
     obj = SCM_MEMOIZED_EXP (obj);
-  else if (!scm_is_pair (obj))
-    SCM_WRONG_TYPE_ARG (1, obj);
+
   h = scm_whash_get_handle (scm_source_whash, obj);
   if (SCM_WHASHFOUNDP (h))
     p = SCM_WHASHREF (scm_source_whash, h);



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
>   guile> (define x 'a-symbol)
>   guile> (set-source-properties! x '((a . 2)))
>   ((a . 2))
>   guile> (source-properties x)
>   ((a . 2))

A symbol used in two places is the same object, so file/line attached
by both occurrances will overwrite, or whatever.  Ditto other uniques
like keywords, and immediates like fixnums and chars.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Neil Jerram]

Kevin Ryde <user42@zip.com.au> writes:

> ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>>
>>   guile> (define x 'a-symbol)
>>   guile> (set-source-properties! x '((a . 2)))
>>   ((a . 2))
>>   guile> (source-properties x)
>>   ((a . 2))
>
> A symbol used in two places is the same object, so file/line attached
> by both occurrances will overwrite, or whatever.  Ditto other uniques
> like keywords, and immediates like fixnums and chars.

Indeed.  What was your motivation for wanting to attach source
properties to non-pairs, though?

      Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Neil Jerram <neil@ossau.uklinux.net> writes:

> Kevin Ryde <user42@zip.com.au> writes:

>> A symbol used in two places is the same object, so file/line attached
>> by both occurrances will overwrite, or whatever.  Ditto other uniques
>> like keywords, and immediates like fixnums and chars.

Symbols and strings are different from immediates.  What you say is true
for symbols, not for strings:

  guile> (define x 'sym)
  guile> (define y 'sym)
  guile> (set-source-properties! x '((hello . world)))
  ((hello . world))
  guile> (source-properties y)
  ((hello . world))

  guile> (define x "str")
  guile> (define y "str")
  guile> (set-source-properties! x '((hello . world)))
  ((hello . world))
  guile> (source-properties y)
  ()

This is because (1) `source-properties' looks up objects using
`hashq-ref' (and, therefore, `eq?'), and (2) because `eq?' behaves
differently on symbols than on strings (quoting R5RS):

   `Eq?' and `eqv?' are guaranteed to have the same behavior on
   symbols, booleans, the empty list, pairs, procedures, and non-empty
   strings and vectors.

In particular, `(eq? "a" "a")' returns `#f' with Guile.

> Indeed.  What was your motivation for wanting to attach source
> properties to non-pairs, though?

Well, the manual doesn't mention this restriction and, rather than
fixing the manual, I wanted to understand the rationale behind this
restriction.

I see no reason not to allow source properties to be attached to
arbitrary non-immediates.  The fact that this "won't work" for symbols
is not, IMO, a sufficiently good reason to the current restriction.  For
instance, one might want to have the following definition of `eval':

  (let ((real-eval eval))
    (set! eval
          (lambda (expr env)
            (let ((props (source-properties expr))
                  (result (real-eval expr env)))
              (if (or (vector? result) (record? result) (pair? result))
                  (set-source-properties! result props))
              result))))

This allows to keep source information further at run-time.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Well, the manual doesn't mention this restriction and, rather than
> fixing the manual, I wanted to understand the rationale behind this
> restriction.

That's fair enough.  I guess the rationale is that the unit of
evaluation (as presented in backtraces for example) is a list, so it
is useful for source properties to be stored on lists when those are
read.

> I see no reason not to allow source properties to be attached to
> arbitrary non-immediates.  The fact that this "won't work" for symbols
> is not, IMO, a sufficiently good reason to the current restriction.  For
> instance, one might want to have the following definition of `eval':
>
>   (let ((real-eval eval))
>     (set! eval
>           (lambda (expr env)
>             (let ((props (source-properties expr))
>                   (result (real-eval expr env)))
>               (if (or (vector? result) (record? result) (pair? result))
>                   (set-source-properties! result props))
>               result))))
>
> This allows to keep source information further at run-time.

Yes, but why is that useful?

(So far, I think I'd vote for fixing the manual rather than extending
source properties ...)

       Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Kevin Ryde]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
> In particular, `(eq? "a" "a")' returns `#f' with Guile.

But cf currently just one empty string,

	(eq? "" "") => #t


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Kevin Ryde <user42@zip.com.au> writes:

> But cf currently just one empty string,
>
> 	(eq? "" "") => #t

This also is made compulsory by R5RS.

Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> That's fair enough.  I guess the rationale is that the unit of
> evaluation (as presented in backtraces for example) is a list, so it
> is useful for source properties to be stored on lists when those are
> read.

Sure.

> Yes, but why is that useful?

Why is it useless?  ;-)

I found it useful in a project that evaluates source in several steps:

  read [sexp] -> convert to alternate representation -> write things

Errors may occur during the last stage.  However, the user doesn't care
about the intermediate stage: they just want to know how the errors
occurring in the last stage relate to its source.  Therefore, source
information needs to be "piggy-backed" all along the process.

In any case, it's up to the user to decide what's useful and what's not.
Guile is here to implement mechanisms, not policy.  If we were to choose
the status-quo, then I'd have to implement a very similar mechanism by
myself, just because the one provided by Guile is unnecessarily
over-specific.

> (So far, I think I'd vote for fixing the manual rather than extending
> source properties ...)

I consider the restriction to pairs arbitrary (but I do understand that
it doesn't harm given the way it is currently used).  What's wrong with
removing such arbitrary restrictions?

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Hi,
>
> Neil Jerram <neil@ossau.uklinux.net> writes:
>
>> Yes, but why is that useful?
>
> Why is it useless?  ;-)

I didn't say it was!  I just wanted you to describe your motivation in
more concrete terms.

There are a couple of reasons why it seems obvious to me that
extending source properties is a bad idea, but which may not be
obvious generally.

1. Source properties are used for very specific purposes by libguile
   and on performance-critical paths:

   - when reading code using the built-in reader, so that the
     information can contribute later to the (also built-in) backtrace
     function

   - in the low level implementation of breakpoints, when deciding
     whether to call an evaluator trap handler.

   Adding stuff to scm_source_whash which doesn't need to be there is
   not going to help these paths.

2. All of the old property interfaces (source-properties,
   object-properties and procedure-properties) are considered to be
   not very nice, and would all be deprecated but for the fact that
   they are still used for some important bits of libguile
   infrastructure (such as (1) and low-level tracing).

   The recommended way to handle properties in new code is with
   make-object-property.

So unless you are wanting specifically to hook in to the mechanisms
that currently rely on source properties, it's a bad idea to use
them.

> I found it useful in a project that evaluates source in several steps:
>
>   read [sexp] -> convert to alternate representation -> write things
>
> Errors may occur during the last stage.  However, the user doesn't care
> about the intermediate stage: they just want to know how the errors
> occurring in the last stage relate to its source.  Therefore, source
> information needs to be "piggy-backed" all along the process.
>
> In any case, it's up to the user to decide what's useful and what's not.
> Guile is here to implement mechanisms, not policy.

Yes, but in this case the mechanism is make-object-property, and
AFAICT that would meet your need just fine.

Regards,
        Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Hi,

Neil Jerram <neil@ossau.uklinux.net> writes:

> I didn't say it was!  I just wanted you to describe your motivation in
> more concrete terms.

I actually only partly describe my motivations.  ;-)

My original motivation was to implement "position recording" in
`guile-reader'.  In order to be compatible with the built-in reader, I
wanted to use the same mechanism as the one it uses.  When I initially
implemented it (not knowing about the restriction), I wrote something
like:

  if (SCM_NIMP (expr))
    {
      scm_set_source_property_x (expr, scm_sym_line, line);
      ...
    }

And I was surprised to see that this wouldn't work on any kind of
non-immediate.  I considered this an /arbitrary/ limitation: why would
my reader record positions only on pairs?

> There are a couple of reasons why it seems obvious to me that
> extending source properties is a bad idea, but which may not be
> obvious generally.
>
> 1. Source properties are used for very specific purposes by libguile
>    and on performance-critical paths:
>
>    - when reading code using the built-in reader, so that the
>      information can contribute later to the (also built-in) backtrace
>      function
>
>    - in the low level implementation of breakpoints, when deciding
>      whether to call an evaluator trap handler.
>
>    Adding stuff to scm_source_whash which doesn't need to be there is
>    not going to help these paths.

Right, source properties are not "one size fits all" and should only be
used by the reader.  Perhaps we should add a line about this in the
manual?

> 2. All of the old property interfaces (source-properties,
>    object-properties and procedure-properties) are considered to be
>    not very nice, and would all be deprecated but for the fact that
>    they are still used for some important bits of libguile
>    infrastructure (such as (1) and low-level tracing).
>
>    The recommended way to handle properties in new code is with
>    make-object-property.
>
> So unless you are wanting specifically to hook in to the mechanisms
> that currently rely on source properties, it's a bad idea to use
> them.

Right.  Indeed, in my previous example, I should probably use
`make-object-property' instead.

Still, that doesn't make this restriction rational.  ;-)

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Neil Jerram]

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> My original motivation was to implement "position recording" in
> `guile-reader'.  In order to be compatible with the built-in reader, I
> wanted to use the same mechanism as the one it uses.  When I initially
> implemented it (not knowing about the restriction), I wrote something
> like:
>
>   if (SCM_NIMP (expr))
>     {
>       scm_set_source_property_x (expr, scm_sym_line, line);
>       ...
>     }
>
> And I was surprised to see that this wouldn't work on any kind of
> non-immediate.  [...]  Right, source properties are not "one size
> fits all" and should only be used by the reader.  Perhaps we should
> add a line about this in the manual?

Yes, I'll look into doing that.

     Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Neil Jerram]

Neil Jerram <neil@ossau.uklinux.net> writes:

> ludovic.courtes@laas.fr (Ludovic Courtès) writes:
>
>> And I was surprised to see that this wouldn't work on any kind of
>> non-immediate.  [...]  Right, source properties are not "one size
>> fits all" and should only be used by the reader.  Perhaps we should
>> add a line about this in the manual?
>
> Yes, I'll look into doing that.

Now in CVS:

2005-10-15  Neil Jerram  <neil@ossau.uklinux.net>

	* api-debug.texi (Source Properties): Add text describing/advising
	limited use of source properties.

	* api-debug.texi (Source Properties): Documentation of source
	property procedures moved here from ...

	* api-procedures.texi (Procedure Properties): ... where it didn't
	belong.

Regards,
        Neil



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: [PATCH] Source properties on arbitrary non-immediate values

[Ludovic Courtès]

Neil Jerram <neil@ossau.uklinux.net> writes:

> Now in CVS:

Ok, cool.

Thanks,
Ludovic.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel