Re: Commercial development

Re: Commercial development

[Neil Jerram]

Scott G. Miller wrote:
> 
> For one, you have to execute a non-standard statement to load the R5RS 
> macro system. 

Well, given a file (which you only have to write once) containing
-----------8<---------------
(use-modules (ice-9 r5rs))
(use-syntax (ice-9 syntax))
-----------8<---------------
you can run Guile as "guile -l <above file> ..." to get an R5RS 
interpreter.  (Subject to the following remaining bugs, of course.)

> But beyond that, try running 
> http://sisc.sf.net/r5rs_pitfall.scm on Guile.  At least on 1.6.7 it
> still has problems.  Maybe 1.8 will be better.

Yes, I see.  But 1.8 will indeed be better; the current CVS only has 
problems with
- 1.1 - for which I have a candidate fix
- 8.3 - which the comment in r5rs_pitfall.scm suggests is debatable
- the final map/call/cc test, where Guile gives "not call/cc safe, but 
probably tail recursive and efficient", and which the comment suggests 
is acceptable.

> 
> 	Scott

Thanks!
	Neil


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

Re: Commercial development

[Neil Jerram]

Neil Jerram wrote:
> Scott G. Miller wrote:
> 
>>But beyond that, try running 
>>http://sisc.sf.net/r5rs_pitfall.scm on Guile.  At least on 1.6.7 it
>>still has problems.  Maybe 1.8 will be better.
> 
> Yes, I see.  But 1.8 will indeed be better; the current CVS only has 
> problems with
> - 1.1 - for which I have a candidate fix

Is this a good fix for r5rs_pitfall 1.1?  The possible issue that I can 
see is performance, but I think it's unlikely that a letrec will have 
enough bindings, or with deep enough structure in the <inits>, for this 
to become significant.

RCS file: /cvsroot/guile/guile/guile-core/libguile/eval.c,v
retrieving revision 1.394
diff -u -r1.394 eval.c
--- eval.c	30 Mar 2005 18:43:49 -0000	1.394
+++ eval.c	20 Apr 2005 19:04:15 -0000
@@ -3570,7 +3570,7 @@
                  init_forms = SCM_CDR (init_forms);
                }
              while (!scm_is_null (init_forms));
-            SCM_SETCDR (SCM_CAR (env), init_values);
+            SCM_SETCDR (SCM_CAR (env), scm_copy_tree (init_values));
            }
            x = SCM_CDR (x);
            PREP_APPLY (SCM_UNDEFINED, SCM_EOL);

Regards,
	Neil


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

Re: Commercial development

[Marius Vollmer]

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

> Is this a good fix for r5rs_pitfall 1.1?

scm_copy_tree is too much, since it makes the following code fail:

   (let ((a '(1 2 3))) (letrec ((x a) (y a)) (eq? x y)))

We should not copy the values themselves, just the storage cells;
thus, scm_list_copy is the right thing here.

But should we make that change?  I am not sure.  It causes some
significant run-time overhead for an obscure benefit.  For now I'll
just put in some comments in eval.c and r5rs_pitfall.test.

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


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

Re: Commercial development

[Neil Jerram]

Marius Vollmer wrote:
> 
> scm_copy_tree is too much, since it makes the following code fail:
> 
>    (let ((a '(1 2 3))) (letrec ((x a) (y a)) (eq? x y)))
> 
> We should not copy the values themselves, just the storage cells;
> thus, scm_list_copy is the right thing here.

Yes, I see that now.

> But should we make that change?  I am not sure.  It causes some
> significant run-time overhead for an obscure benefit.  For now I'll
> just put in some comments in eval.c and r5rs_pitfall.test.

I'm not sure it's that obscure, and it seems a shame to leave this as
ammunition for those who like to claim that Guile is not R5RS-compliant.

Is it the performance you are concerned about, or the extra consing?
Surely there must be other things we do for "correct Scheme behaviour",
that have a similar cost to a scm_list_copy?

Two further thoughts that could help us here:

(1) If the environment could be made copy-on-write, we could do the copy
only when actually needed.  (But I have no idea whether this is feasible.)

(2) We could provide a 'strict-r5rs option, and use it to decide whether
to do the copy.  (In practice, 'strict-r5rs would probably only be on by
default in Guile's implementation of the SRFI-22 scheme-r5rs interpreter.)

Regards,
	Neil


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

Re: Commercial development

[Neil Jerram]

Neil Jerram wrote:
> 
> I'm not sure it's that obscure, and it seems a shame to leave this as
> ammunition for those who like to claim that Guile is not R5RS-compliant.

Another possibility would be to do what SCM does.  If I understand
correctly, SCM does the list construction that we have noted as being
necessary to pass the test, but compensates by not building the init
values into a list to begin with.

Instead, it builds the init values onto the stack, by using a recursive
function like this:

static void ecache_evalx(x)
     SCM x;
{
  SCM argv[10];
  int i = 0, imax = sizeof(argv)/sizeof(SCM);
  scm_env_tmp = EOL;
  while NIMP(x) {
    if (imax==i) {
      ecache_evalx(x);
      break;
    }
    argv[i++] = EVALCAR(x);
    x = CDR(x);
  }
  ENV_V2LST((long)i, argv);
}

The ENV_V2LST at the end then conses all the values on the stack into a
list, and the calling code finishes things off by inserting that list
into the extended environment.

Cunning, isn't it?  It passes the test because a continuation captured
during an ecache_evalx call will preserve the values on the stack, and
because the values on the stack are not affected by set!s on the letrec
environment.

What do you think?  It seems like a very self-contained trick, so I
think we could easily copy it to Guile.

	Neil


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

Re: Commercial development

[Marius Vollmer]

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

> Cunning, isn't it?  It passes the test because a continuation captured
> during an ecache_evalx call will preserve the values on the stack, and
> because the values on the stack are not affected by set!s on the letrec
> environment.

Yep, I was thinkning in this direction as well... looks like the right
approach.

> What do you think?  It seems like a very self-contained trick, so I
> think we could easily copy it to Guile.

The trick: yes; the code: probably not.  Neil, do you want to do it?

-- 
GPG: D5D4E405 - 2F9B BCCC 8527 692A 04E3  331E FAF8 226A D5D4 E405


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

Re: Commercial development

[Neil Jerram]

Marius Vollmer wrote:
> Neil Jerram <neil@ossau.uklinux.net> writes:
> 
> 
>>Cunning, isn't it?  It passes the test because a continuation captured
>>during an ecache_evalx call will preserve the values on the stack, and
>>because the values on the stack are not affected by set!s on the letrec
>>environment.
> 
> 
> Yep, I was thinkning in this direction as well... looks like the right
> approach.
> 
> 
>>What do you think?  It seems like a very self-contained trick, so I
>>think we could easily copy it to Guile.
> 
> 
> The trick: yes; the code: probably not.  Neil, do you want to do it?
> 
Yes, I'll have a go.

	Neil


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

Re: Commercial development

[Neil Jerram]

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

Neil Jerram wrote:
> 
> Yes, I'll have a go.

How about the attached?  The scm_reverse_x is annoying, but removing it
would require

- either modifying transform_bindings etc. so as not to reverse things
in the first place, or so as to reverse the init vars in the same way as
the init forms

- or constructing the list backwards in eval_letrec_inits - but I can't
see a way of doing that.

	Neil


[-- Attachment #2: eval.c.diff --]
[-- Type: text/x-patch, Size: 1997 bytes --]

Index: eval.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/eval.c,v
retrieving revision 1.397
diff -u -r1.397 eval.c
--- eval.c	6 Jun 2005 18:49:55 -0000	1.397
+++ eval.c	12 Jun 2005 09:30:14 -0000
@@ -96,6 +96,7 @@
 static SCM canonicalize_define (SCM expr);
 static SCM *scm_lookupcar1 (SCM vloc, SCM genv, int check);
 static SCM unmemoize_builtin_macro (SCM expr, SCM env);
+static SCM eval_letrec_inits (SCM env, SCM init_forms, SCM init_values);
 
 \f
 
@@ -3148,6 +3149,29 @@
   return *results;
 }
 
+static SCM
+eval_letrec_inits (SCM env, SCM init_forms, SCM init_values)
+{
+  SCM argv[10];
+  int i = 0, imax = sizeof (argv) / sizeof (SCM);
+
+  while (!scm_is_null (init_forms))
+    {
+      if (imax == i)
+	{
+	  init_values = eval_letrec_inits (env, init_forms, init_values);
+	  break;
+	}
+      argv[i++] = EVALCAR (init_forms, env);
+      init_forms = SCM_CDR (init_forms);
+    }
+
+  for (i--; i >= 0; i--)
+    init_values = scm_cons (argv[i], init_values);
+
+  return init_values;
+}
+
 #endif /* !DEVAL */
 
 
@@ -3563,21 +3587,9 @@
           x = SCM_CDR (x);
           {
             SCM init_forms = SCM_CAR (x);
-            SCM init_values = SCM_EOL;
-            do
-              {
-                init_values = scm_cons (EVALCAR (init_forms, env), init_values);
-                init_forms = SCM_CDR (init_forms);
-              }
-            while (!scm_is_null (init_forms));
-
-	    /* In order to make case 1.1 of the R5RS pitfall testsuite
-	       succeed, we would need to copy init_values here like
-	       so:
-
-	       init_values = scm_list_copy (init_values);
-	    */
-            SCM_SETCDR (SCM_CAR (env), init_values);
+	    SCM init_values = eval_letrec_inits (env, init_forms, SCM_EOL);
+            SCM_SETCDR (SCM_CAR (env),
+			scm_reverse_x (init_values, SCM_UNDEFINED));
           }
           x = SCM_CDR (x);
           PREP_APPLY (SCM_UNDEFINED, SCM_EOL);

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

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

Re: Commercial development

[Neil Jerram]

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

Neil Jerram wrote:
> How about the attached?  The scm_reverse_x is annoying, but removing it
> would require [...] constructing the list backwards in eval_letrec_inits - but I can't
> see a way of doing that.

I worked the list construction out at last, so I think the attached is
good to go now.  OK to commit?

	Neil


[-- Attachment #2: eval-letrec-inits.diff --]
[-- Type: text/x-patch, Size: 4918 bytes --]

cvs server: Diffing libguile
Index: libguile/ChangeLog
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/ChangeLog,v
retrieving revision 1.2293
diff -u -u -r1.2293 ChangeLog
--- libguile/ChangeLog	18 Jul 2005 13:55:44 -0000	1.2293
+++ libguile/ChangeLog	18 Jul 2005 20:49:06 -0000
@@ -1,3 +1,8 @@
+2005-07-18  Neil Jerram  <neil@ossau.uklinux.net>
+
+	* eval.c (eval_letrec_inits): New.
+	(CEVAL): Eval letrec initializer forms using eval_letrec_inits.
+
 2005-07-18  Mikael Djurfeldt  <mdj@d14n36.pdc.kth.se>
 
 	Some changes towards making it possible to run Guile on the EM64T
Index: libguile/eval.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/eval.c,v
retrieving revision 1.398
diff -u -u -r1.398 eval.c
--- libguile/eval.c	12 Jul 2005 00:28:09 -0000	1.398
+++ libguile/eval.c	18 Jul 2005 20:49:08 -0000
@@ -96,6 +96,7 @@
 static SCM canonicalize_define (SCM expr);
 static SCM *scm_lookupcar1 (SCM vloc, SCM genv, int check);
 static SCM unmemoize_builtin_macro (SCM expr, SCM env);
+static void eval_letrec_inits (SCM env, SCM init_forms, SCM **init_values_eol);
 
 \f
 
@@ -3148,6 +3149,30 @@
   return *results;
 }
 
+static void
+eval_letrec_inits (SCM env, SCM init_forms, SCM **init_values_eol)
+{
+  SCM argv[10];
+  int i = 0, imax = sizeof (argv) / sizeof (SCM);
+
+  while (!scm_is_null (init_forms))
+    {
+      if (imax == i)
+	{
+	  eval_letrec_inits (env, init_forms, init_values_eol);
+	  break;
+	}
+      argv[i++] = EVALCAR (init_forms, env);
+      init_forms = SCM_CDR (init_forms);
+    }
+
+  for (i--; i >= 0; i--)
+    {
+      **init_values_eol = scm_list_1 (argv[i]);
+      *init_values_eol = SCM_CDRLOC (**init_values_eol);
+    }
+}
+
 #endif /* !DEVAL */
 
 
@@ -3563,21 +3588,10 @@
           x = SCM_CDR (x);
           {
             SCM init_forms = SCM_CAR (x);
-            SCM init_values = SCM_EOL;
-            do
-              {
-                init_values = scm_cons (EVALCAR (init_forms, env), init_values);
-                init_forms = SCM_CDR (init_forms);
-              }
-            while (!scm_is_null (init_forms));
-
-	    /* In order to make case 1.1 of the R5RS pitfall testsuite
-	       succeed, we would need to copy init_values here like
-	       so:
-
-	       init_values = scm_list_copy (init_values);
-	    */
-            SCM_SETCDR (SCM_CAR (env), init_values);
+	    SCM init_values = scm_list_1 (SCM_BOOL_T);
+	    SCM *init_values_eol = SCM_CDRLOC (init_values);
+	    eval_letrec_inits (env, init_forms, &init_values_eol);
+            SCM_SETCDR (SCM_CAR (env), SCM_CDR (init_values));
           }
           x = SCM_CDR (x);
           PREP_APPLY (SCM_UNDEFINED, SCM_EOL);
cvs server: Diffing libguile-ltdl
cvs server: Diffing libguile-ltdl/upstream
cvs server: Diffing libltdl
cvs server: Diffing oop
cvs server: Diffing oop/goops
cvs server: Diffing qt
cvs server: Diffing qt/md
cvs server: Diffing qt/time
cvs server: Diffing scripts
cvs server: Diffing srfi
cvs server: Diffing test-suite
Index: test-suite/ChangeLog
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/test-suite/ChangeLog,v
retrieving revision 1.354
diff -u -u -r1.354 ChangeLog
--- test-suite/ChangeLog	12 Jun 2005 12:31:52 -0000	1.354
+++ test-suite/ChangeLog	18 Jul 2005 20:49:08 -0000
@@ -1,3 +1,7 @@
+2005-07-18  Neil Jerram  <neil@ossau.uklinux.net>
+
+	* tests/r5rs_pitfall.test (1.1): Now passes.
+
 2005-06-12  Marius Vollmer  <mvo@zagadka.de>
 
 	* standalone/test-gh.c: Do nothing when deprecated things are
cvs server: Diffing test-suite/standalone
cvs server: Diffing test-suite/tests
Index: test-suite/tests/r5rs_pitfall.test
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/test-suite/tests/r5rs_pitfall.test,v
retrieving revision 1.6
diff -u -u -r1.6 r5rs_pitfall.test
--- test-suite/tests/r5rs_pitfall.test	5 Jun 2005 20:54:19 -0000	1.6
+++ test-suite/tests/r5rs_pitfall.test	18 Jul 2005 20:49:08 -0000
@@ -18,8 +18,6 @@
 ;; These tests have been copied from
 ;; http://sisc.sourceforge.net/r5rs_pitfall.scm and the 'should-be'
 ;; macro has been modified to fit into our test suite machinery.
-;;
-;; Test 1.1 fails, but we expect that.
 
 (define-module (test-suite test-r5rs-pitfall)
   :use-syntax (ice-9 syncase)
@@ -48,9 +46,7 @@
 ;; defines in letrec body 
 ;; http://groups.google.com/groups?selm=87bsoq0wfk.fsf%40app.dial.idiom.com
 
-;; See eval.c for how to make this test succeed.  Look for "r5rs pitfall".
-
-(should-be-but-isnt 1.1 0
+(should-be 1.1 0
  (let ((cont #f))
    (letrec ((x (call-with-current-continuation (lambda (c) (set! cont c) 0)))
             (y (call-with-current-continuation (lambda (c) (set! cont c) 0))))
cvs server: Diffing test-suite/tests/c-api

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

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