-
Notifications
You must be signed in to change notification settings - Fork 40
Fix quote & region scope problems #368
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
Fix quote & region scope problems #368
Conversation
LPTK
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
| :fixme // FIXME: weird error | ||
|
|
||
| :ge | ||
| `1 `== `2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change these to `===, then.
But it's strange that we can't use a global thing like Predef – that's not truly a cross-stage reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is because we need to compute which files should be included in the next stage. So, we need to use import.meta to get the relative path. In this example, == in Predef shadows the default one. That's why we get the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't get it (see my other message).
Co-authored-by: Lionel Parreaux <lionel.parreaux@gmail.com>
| //│ ) | ||
|
|
||
|
|
||
| // We cannot quote a symbol from another file here due to the lack of `import.meta` in REPL mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would import.meta be needed? All we need is to make the genearted code import Predef, not the current file...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assume that we have a file A.mls containing a quoted symbol that is defined in ./B.mls (a relative path to A.mls). If we generate the quoted code fragment into a file ./C.mls (** a relative path to the pwd, which is the root dir for now **), we must know the relative path of B.mls to C.mls, which requires knowing where A.mls is. In the ES Module, __filename is not defined, and we need to use import.meta to retrieve the full file path of A.mls. However, it is not available in the REPL mode. That's why I did this check. Of course, it is possible to always generate an import statement for Predef. But for now, we are still using a relative path to import Predef. If we can write import "Predef", and the compiler can locate the file by the given paths, then the reference to symbols in Predef will no longer rely on import.meta.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clearly, we should be able to know the path toPredef here, because it is being imported in the code! We know it at compile time, so we should be able to generate appropriate code.
It seems that I'm not authorized to push to the branch #366 directly.
In this PR:
Scoped