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

Should fix #14. Generate #%require at namespace base phase for top-level expressions #17

Merged
merged 1 commit into from
Oct 8, 2020
Merged

Should fix #14. Generate #%require at namespace base phase for top-level expressions #17

merged 1 commit into from
Oct 8, 2020

Conversation

shhyou
Copy link
Contributor

@shhyou shhyou commented Jan 3, 2018

After this patch, the test case posted by @florence in #14 passes. The following program also runs, but I didn't run it in DrRacket.

#lang racket
(require errortrace)
(require (for-template phc-adt))

@mflatt
Copy link
Member

mflatt commented Jan 4, 2018

LGTM

@sorawee
Copy link
Contributor

sorawee commented Aug 2, 2020

@shhyou did you accidentally close the PR?

@shhyou
Copy link
Contributor Author

shhyou commented Aug 2, 2020

I thought the issue is already fixed when reading the commit history at some point, but could be that I just confused myself.

I can look into this again tomorrow.

@sorawee
Copy link
Contributor

sorawee commented Sep 16, 2020

I can confirm that it is not fixed. Running the test that you adds fails with #%require: unbound identifier;

@shhyou shhyou reopened this Sep 17, 2020
When generating require forms for *top-level*
annotated expressions, use the base phase of
the current namespace at which the evaluation
takes place instead of the constant 0 returned
by syntax-local-phase-level.

Fixes #14.
@shhyou
Copy link
Contributor Author

shhyou commented Sep 17, 2020

Thanks for the heads up. I forgot about this and indeed the issue is unfixed. I've updated the pull request.

@sorawee
Copy link
Contributor

sorawee commented Oct 8, 2020

Is it possible to get this merged by the 7.9 release?

@rfindler
Copy link
Member

rfindler commented Oct 8, 2020

@sorawee branch day has already happened (yesterday) but I think it is okay if @mflatt still is happy with the commit.

@mflatt mflatt merged commit 914f26a into racket:master Oct 8, 2020
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.

None yet

4 participants