coderabbitai[bot]
commented
1 month ago Walkthrough
The recent changes introduce significant restructuring and renaming of data types and patterns across multiple modules. Key modifications include splitting TypePattern into TypeCtorPattern and TypePatternVar, renaming ConstructorPatternF to DataCtorPattern, and updating various functions and modules to accommodate these changes. Additionally, new primitives and macros have been added, and error handling has been improved by utilizing non-empty lists. Instances of ShortShow have been removed from several modules.
Changes
| Files | Change Summary |
|---|---|
src/Core.hs, src/Evaluator.hs |
Renamed and restructured data types and patterns, updated functions to reflect these changes. |
src/Expander.hs, src/Expander/Error.hs, src/Expander/Monad.hs, src/Expander/Primitives.hs |
Added new primitives and macros, adjusted existing functions and error handling to use non-empty lists. |
src/Binding.hs, src/Binding/Info.hs, src/Datatype.hs, src/Expander/Task.hs, src/Expander/Syntax.hs, src/Syntax/Syntax.hs, src/Type.hs |
Removed ShortShow instances and related imports. |
tests/Test.hs |
Updated function signatures and type references to reflect new data type structures. |
stdlib/prelude.kl, toy.kl |
Updated datatype definitions and refined macro behaviors. |
Sequence Diagram(s) (Beta)
No sequence diagrams are needed for these changes as they primarily involve renaming, restructuring, and minor functional updates.
Poem
In the code where patterns play,
New names and types now hold sway.
Constructors dance in a fresh array,
Primitives and macros lead the way.
Errors handled with non-empty flair,
ShortShow bids farewell, everywhere.
Hail to the changes, with a rabbit’s care! 🐇✨
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
gelisam
As discussed here, if
my-macrois a user-defined macro whose(-> Syntax (Macro Syntax))ismyMacroImpland(my-macro)is encountered at phase 0, then the current code will evaluate(myMacroImpl '(my-macro))at phase 1 but will execute the resulting(Macro Syntax)at phase 0. This seems wrong to me, I think it should also execute at phase 1.I have changed the code so that the
(Macro Syntax)is also executed at phase 1. The only consequence of this change which was found by the test suite was to breakfree-identifier=?. Consider this code:When the
(free-identifier=? x 'my-keyword)action is executed, then the current implementation will look up "my-keyword" and "my-keyword" in the current phase (previously phase 0, now phase 1) and determine whether they point to the same binding. In phase 0, they do point to the same binding, sofree-identifier=?return true, but at phase 1 there is no binding for "my-keyword", sofree-identifier=?returns false.Now, we do want
free-identifier=?to return true, so I have changedfree-identifier=?so that instead of looking up "my-keyword" in the current phase (now phase 1), it now looks it up in phase 0, thus resolving the issue. This change makes sense to me: if e.g. the macro had a local variable named "my-keyword", we would not wantxto resolve to that local variable, we still wantxto resolve to the top-leveldefine. Sofree-identifier=?should look up the variables in the environment of the macro's call site, not in the current environment.Does this make sense? Did I find and fix a bug, or is my intuition completely wrong?