-
Notifications
You must be signed in to change notification settings - Fork 77
Add missing bits to SAWScript's Cryptol import syntax #2607
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
Conversation
|
||
%token | ||
'import' { TReserved _ "import" } | ||
'submodule' { TReserved _ "submodule" } |
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.
Surely we should have a test case that exercises this new bit of the parser?
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.
We should, but I left it off for now because it's going to interact with Mark's branches. There's basically three options:
-
Merge this now, rebase all of Mark's code on it, add code there to recognize the submodule tag when processing the import, add a test there.
-
Merge Mark's code first, rebase this on it, add code here to recognize the submodule tag when processing the import, add a test here.
-
Merge both independently, then do another branch that adds the code to recognize the submodule tag when processing the import along with a test.
For various reasons (3) seems preferable...
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.
(Contributing factor: nothing new should be needed on Mark's side to be able to handle the qualified names; that was purely a parser limitation)
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.
Ah, I see. Should I understand that this just makes the parser recognize the submodule
syntax, but doesn't actually do anything semantically meaningful with it?
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.
Yes.
f17b797
to
643b427
Compare
- allow the word "submodule" (it is currently ignored, but that will likely change soon) - allow the import name to be qualified (as in "...as M::N") Add a test for the latter part.
The build failure seen earlier today is almost certainly cache corruption. Note that the 1.4 branch is using cache version 11; it's probably best to not try to share.
5e3a428
to
c3f765d
Compare
(this force-push to fix test_sawscript_builtins) |
Closes #2605.
Note that it now accepts
import submodule
and stuffs the existence of the wordsubmodule
into theImport
record, but doesn't do anything with it; @mtullsen I expect you have places to hook that up to in your code.