Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor bugfixes and two new definitions #37

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

DataSnake
Copy link

Fixed a bug in the require statement for racket:random and changed inc and dec to use built-in functions that did the same thing instead of being defined from scratch.

Added apply-in-underlying-scheme because the book's solution of just using the line (define apply-in-underlying-scheme apply) at the start of the metacircular evaluator isn't compatible with how Racket handles definitions.

Changed the way set! is handled so that Exercise 4.45 can be completed successfully. If there's an amb statement active, calls to set! will modify amb-fail so that backtracking will undo assignments. This only kicks in if the amb tree isn't currently empty, so it should have no discernible performance impact on programs that don't use amb operations.

Added the user-initial-environment primitive so that the code for the query system in Section 4.4.4 can be run as-is.

Changed rename-in to only-in, as re-importing the rest of the Racket package would be redundant.
Modified set! so that if there's an amb expression active, backtracking will undo assignments. This is necessary to complete exercise 4.44.
Changed inc and dec to refer to the existing Racket functions add1 and sub1 rather than being redefined from scratch.
sicp/main.rkt Outdated
@@ -24,8 +25,8 @@
(define+provide nil '())
(define+provide the-empty-stream '())
(define+provide stream-null? null?)
(define+provide (inc x) (+ x 1))
(define+provide (dec x) (- x 1))
(define+provide inc add1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We intentionally do not do this because:

(define inc add1)
(println inc) ;=> #<procedure:add1>

prints a name that we do not want.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hadn't thought of that. I guess I can revert that part, then. What about the other changes?

This reverts commit 623ac59.
@sorawee
Copy link
Contributor

sorawee commented Dec 19, 2021

Thanks for the PR!

You said that:

Fixed a bug in the require statement for racket:random

Can I ask what issue did you encounter that make you think that this is a "bug"? I agree that only-in seems more appropriate, but rename-in should work too, right?

I will have to take a look at the book to be sure if other changes look alright.

Let me also ping @soegaard in case he sees anything.

@DataSnake
Copy link
Author

DataSnake commented Dec 19, 2021

Can I ask what issue did you encounter that make you think that this is a "bug"? I agree that only-in seems more appropriate, but rename-in should work too, right?

When you open main.rkt in DrRacket with the rename-in line in place, hovering the mouse over anything imported from racket will draw arrows to both that line and the #lang racket line at the top, and the tooltip will say that it has two binding occurrences. I was curious about that, so I looked at it with the macro stepper, and (require (rename-in racket [random racket:random])) expands to (#%require (all-except racket random) (rename racket racket:random random)). It still works as far as I can tell, but it seems wasteful to re-import a bunch of things that were already being provided by the language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants