single_cream icon indicating copy to clipboard operation
single_cream copied to clipboard

problem with set!

Open wboeke opened this issue 6 years ago • 3 comments

The following code doesn't work:

(define n 5)
(set! n 6)
scheme_builtin_set: failure looking up [n]

wboeke avatar Jan 30 '19 08:01 wboeke

Thank you for taking the time to experiment and find this flaw!

I never really tested SET! it was added at the very end as an afterthought and just a means to implement LETREC - it only mutates variables in the local environment so

(let ((n 5))
 (set! n 6)
 n)

works but this doesn't.

We could add a bit of code in here to search the globals list if the locals list doesn't have it: https://github.com/rain-1/single_cream/blob/master/src/sch3.c#L1044

or maybe we can avoid having two separate lists all together? That sounds nicer

rain-1 avatar Jan 30 '19 10:01 rain-1

This patch unifies the two environments into one, which makes SET! work on globals.

The downside is that all functions must be defined in order of use. We can't use mutually recursive toplevel definitions anymore.

The first time that comes up is in the minikanren tests.

From daff2c1d0ffbbfdb1be547717aa79383f7394c5a Mon Sep 17 00:00:00 2001
From: rain <[email protected]>
Date: Thu, 31 Jan 2019 15:49:08 +0000
Subject: [PATCH] unify globals and env


diff --git a/src/sch3.c b/src/sch3.c
index 51fe7eb..0f716af 100644
--- a/src/sch3.c
+++ b/src/sch3.c
@@ -1006,9 +1006,6 @@ eval:
 	
 	if(exp->tag == TAG_SYMBOL) {
 		res = scheme_assoc(exp, env);
-		if(res.tag == TAG_FALSE) {
-			res = scheme_assoc(exp, &globals);
-		}
 		if(res.tag == TAG_FALSE) {
 			fprintf(stderr, "error in scheme_eval: reference to an undefined variable [%s].\n", scheme_symbol_name(exp->symbol));
 			exit(1);
@@ -1231,7 +1228,7 @@ struct Obj scheme_curry_definition(struct Obj *exp) {
 	return t3;
 }
 
-struct Obj scheme_exec(struct Obj *exp, struct Obj *env, int display_result) {
+struct Obj scheme_exec(struct Obj *exp, int display_result) {
 	struct Obj t1, t2;
 	struct Obj res;
 	
@@ -1252,18 +1249,22 @@ define_loop:
 		t1 = *exp->cons.cdr->cons.car;
 		t2 = *exp->cons.cdr->cons.cdr;
 		t2 = scheme_make_begin(&t2);
-		t2 = scheme_eval(&t2, env);
-		t1 = scheme_cons(&t1, &t2);
-		// TODO: overwrite existing
+		
+		t1 = scheme_cons(&t1, NULL);
+		// add a dummy binding before evaluating the body, to support recursive definitions
+		// this lets the lambda body have hold of an environment that contains its own binding
 		globals = scheme_cons(&t1, &globals);
 		
+		// SET! the cdr to the new value
+		*t1.cons.cdr = scheme_eval(&t2, &globals);
+		
 		scheme_root_pop();
 		scheme_root_pop();
 		
 		return const_nil;
 	}
 	
-	res = scheme_eval(exp, env);
+	res = scheme_eval(exp, &globals);
 	
 	if(display_result) {
 		scheme_display(stdout, &res, 1);
@@ -1620,9 +1621,8 @@ void scheme_constants_init(void) {
  */
 
 struct Obj preprocess_eval(struct Obj *rt, int display_result) {
-	struct Obj rt2, res;
+	struct Obj res;
 
-	scheme_root_push(&rt2);
 	scheme_root_push(&res);
 
 	*rt = scheme_cons(rt, NULL);
@@ -1633,13 +1633,10 @@ struct Obj preprocess_eval(struct Obj *rt, int display_result) {
 	*rt = scheme_cons(NULL, rt);
 	*rt->cons.car = sym_preprocess;
 	
-	rt2 = const_nil;
-	*rt = scheme_exec(rt, &rt2, 0);
+	*rt = scheme_exec(rt, 0);
 	
-	rt2 = const_nil;
-	res = scheme_exec(rt, &rt2, display_result);
+	res = scheme_exec(rt, display_result);
 	
-	scheme_root_pop();
 	scheme_root_pop();
 	
 	return res;
-- 
2.18.1

rain-1 avatar Jan 31 '19 15:01 rain-1

So the "simplification" of unifying the envs seems like a dead end, we would have to just add the same fix that symbol lookup does (check local env then check global env)

rain-1 avatar Jan 31 '19 15:01 rain-1