-
Notifications
You must be signed in to change notification settings - Fork 398
[ refactor ] ScopedSnocList: Preparing for the long jump (Phase 1) #3512
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
a8c6014 to
f7bc725
Compare
|
Thanks for that. FYI we (including at least @mjustus & me) are planning to do a big code review |
|
@gallais much appreciate! 😇 |
|
@gallais, @mjustus shy reminder about the review and its importance! 🙏 We are going to make further steps of adding parts from Yaffle when this PR and its companion will succeed. All this is a refactor which is important but these steps actually are preparations for something which can be added to the change log . And this step is scheduled to start the development after these 2 phases. So, moving forward here is a corner stone to unlock further Idris improvements. All this is clear for invited people just it seemed important to explicitly emphasize at the message due of questions at Discord group. |
|
We went through about a 3rd of the files. There a semantic issues (e.g. some lists of values being |
|
@gallais any news would be very appreciate! We are in progress of porting other parts from It would be nice if you can put a link to PR/commits/branch/anything where we might participate during the review process. If it is needed of course. |
|
Ping @mjustus We haven't had the opportunity to have another meeting yet. And with |
f7bc725 to
3decb09
Compare
|
Pinned for watching: https://github.com/mjustus/Idris2/tree/scoped-snoc-list-preparing-review |
e5faef5 to
34ebbf0
Compare
|
We met up today (thanks Justus for your hospitality!) and we've made good progress. |
34ebbf0 to
c3a8909
Compare
|
I have pushed your current state to branch snoc-list-preparing-force-pushed on my copy of the repository and am about to revert this branch to f7bc725 because it was getting too difficult reviewing a moving target with large merge conflict. |
c3a8909 to
f7bc725
Compare
|
Pinned for watching: GulinSS#19 |
|
Let's see if it builds |
96ee709 to
7762df0
Compare
Core idea is to split the process of transition from List-based Context on SnocList-based Context on 2 phases: 1. Apply a refactor which does not make any transition (even no observable changes for Idris users) actually but prepare the code for future update. 2. Roll out the transition with limited code changes invasion because it was partially done at the step before. Co-authored-by: Viktor Yudov <me@spcfox.com>
|
A moment, rebase on |
…eview) 1. Reverted `Scopeables` that act like spines (e.g. `NTCon`) because we consider it to be a separate refactoring 2. Disambiguated overloaded `ScopeEmpty` (e.g. `Scope` and `Env`) using namespaces. 3. Identified and replaced instances of `All` quantifiers. Both `List` and `SnocList` enjoy all, which will make future changes easier. 4. Highlighted opportunities for rewrite-free implementations using fish-based (`<><`) sliding windows. 5. Left a bunch of `TODO`s for unrelated refactoring opportunities.
7762df0 to
8455d9d
Compare
|
Sounds like we can begin the merge. After that I will create appropriate issues with links to code (and may be file a new PR with TODO removal but not sure). |
…dris-lang#3512) Co-authored-by: Viktor Yudov <me@spcfox.com> Co-authored-by: Justus Matthiesen <mail@justusmatthiesen.com>
…dris-lang#3512) Co-authored-by: Viktor Yudov <me@spcfox.com> Co-authored-by: Justus Matthiesen <mail@justusmatthiesen.com>
* [ cleanup ] Remove redudant `SnocList` import from #3512 * [ cleanup ] Remove redundant imports * [ cleanup ] Add public imports * [ cleanup ] Small cleanup pretty printers * [ fix ] Remove `%inline` from `sequence` * [ fix ] missing import --------- Co-authored-by: G. Allais <guillaume.allais@ens-lyon.org>
Core idea is to split the process of transition from List-based Scope on SnocList-based Scope on 2 phases: