-
Notifications
You must be signed in to change notification settings - Fork 379
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
[ refactor ] ScopedSnocList: WIP #3368
base: main
Are you sure you want to change the base?
Conversation
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 haven't looked at the whole PR but I have tagged a couple of issues.
This is why I was concerned: there's a lot of domain-specific knowledge
in the choice to represent something as a list vs. a snoclist.
src/Core/Case/CaseTree.idr
Outdated
||| Constructor for a data type; bind the arguments and subterms. | ||
ConCase : Name -> (tag : Int) -> (args : List Name) -> | ||
CaseTree (args ++ vars) -> CaseAlt vars | ||
ConCase : Name -> (tag : Int) -> (args : ScopedList Name) -> |
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's debatable whether the constructor arguments should be listed right-to-left.
Also they would become the most local variables so the nested case tree would
have a type of the form CaseTree (vars <>< args)
rather than args
flowing into
the global scope.
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.
Left for now as marked as you said it is a debatable question.
src/Core/Case/CaseTree.idr
Outdated
insertCaseAltNames p q (ConCase x tag args ct) | ||
= ConCase x tag args | ||
(rewrite appendAssociative args outer (ns ++ inner) in | ||
(rewrite appendAssociative args outer (ns +%+ inner) in |
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.
appendAssociative
is still List-specific
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.
Yep, current step is only renaming to mark existing usages of List
in context of Scope
. No structural changes for current step. Or I didn't get you correctly?
c486736
to
14b674a
Compare
Took into consideration @gallais points where I did replacements where it should not be. Also asked @buzden how to determine where such replacements should be and should not be. He suggested to everytime take a look at https://github.com/edwinb/Yaffle/ for reference when I am getting stuck with questions does it should be @gallais, I continue my work. |
14b674a
to
4090ac5
Compare
Well, I am in progress
|
9b9e8d5
to
afd19ee
Compare
It compiles! The next steps are to
Starting work on the grouping. |
afd19ee
to
7098a37
Compare
Stuck tests running. The difference between the commit and main: % diff tests/build/exec/runtests_app/runtests.ss tests/build/exec/runtests_app/runtests.ss.back
3c3
< ;; @generated by Idris 0.7.0-7098a379f, Chez backend
---
> ;; @generated by Idris 0.7.0-2482ebb43, Chez backend
654,655c654,655
< (define PrimIO-unsafeCreateWorld (lambda (arg-1) (arg-1 )))
< (define PrimIO-unsafePerformIO (lambda (arg-1) (PrimIO-unsafeCreateWorld (lambda (u--w) (let ((eff-0 (arg-1 ))) eff-0)))))
---
> (define PrimIO-unsafeCreateWorld (lambda (arg-1) (arg-1 #f)))
> (define PrimIO-unsafePerformIO (lambda (arg-1) (PrimIO-unsafeCreateWorld (lambda (u--w) (let ((eff-0 (arg-1 u--w))) eff-0)))))
688c688
<
---
> For some reason |
7098a37
to
ef4d1fa
Compare
ef4d1fa
to
4d07a67
Compare
Added special inverted operators for current
|
6a2ca50
to
f50ee80
Compare
5a9881e
to
a0e5246
Compare
db9b3fd
to
5089a92
Compare
8c38ac6
to
db2677c
Compare
…ithFC` and `applySpineWithFC`
ace9719
to
91493da
Compare
91493da
to
2515ede
Compare
…ave `normalizeHoles` for easier logging analysis
2515ede
to
3b5f5d2
Compare
Description
We are going to take again a turn to solve TODO, related to the usage of SnocList at Scope. This change is heavily intrusive and its older attempt was already here.
By suggestion of @buzden we are going to start this heavy change from a very conceptually simple step: collect all usages and mark them. It would help us to schedule further changes at closed volume of overall affected codebase.
Should this change go in the CHANGELOG?
implementation, I have updated
CHANGELOG_NEXT.md
(and potentially alsoCONTRIBUTORS.md
).