Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Direct translation from MicroJuvix to MiniC #1386

Merged
merged 3 commits into from
Jul 19, 2022
Merged

Direct translation from MicroJuvix to MiniC #1386

merged 3 commits into from
Jul 19, 2022

Conversation

lukaszcz
Copy link
Collaborator

@lukaszcz lukaszcz commented Jul 15, 2022 •
edited
Loading

Direct translation from MicroJuvix to MiniC.

Description

Checklist:

  • My code follows the style guidelines of this project
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works:
    • Negative tests
    • Positive tests
    • Shell tests

I added positive tests for #1338 and #1339. Existing tests of MiniC compilation should be enough for the rest.

@janmasrovira janmasrovira self-requested a review July 15, 2022 11:19
@jonaprieto jonaprieto added the enhancement New feature or request label Jul 15, 2022
@jonaprieto jonaprieto added this to the 0.2.2 milestone Jul 15, 2022
@jonaprieto jonaprieto linked an issue Jul 15, 2022 that may be closed by this pull request
Remove the MonoJuvix intermediate language #1341
Closed
@lukaszcz lukaszcz force-pushed the remove-monojuvix branch 3 times, most recently from 663d791 to da96c4a Compare July 15, 2022 11:59
@lukaszcz
Copy link
Collaborator Author

I don't know why the tests went through. They should fail without #8 merged. Without removing quickSort they used to fail for me.

This was referenced Jul 15, 2022
Closed
Closed
Closed
@lukaszcz
Copy link
Collaborator Author

I don't know why the tests went through. They should fail without #8 merged. Without removing quickSort they used to fail for me.

NB: This might be due to a difference in C compiler versions.

This was linked to issues Jul 18, 2022
Polymorphic axiom results in an exception #1338
Closed
Compilation raises an exception when a type parameter occurs only in the target #1339
Closed
janmasrovira
janmasrovira requested changes Jul 18, 2022
View reviewed changes
src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
src/Juvix/Syntax/MicroJuvix/Language.hs Outdated Show resolved Hide resolved
src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
src/Juvix/Translation/MicroJuvixToMiniC/Base.hs Outdated Show resolved Hide resolved
src/Juvix/Translation/MicroJuvixToMiniC/Base.hs Outdated Show resolved Hide resolved
src/Juvix/Translation/MicroJuvixToMiniC/Base.hs Outdated Show resolved Hide resolved
src/Juvix/Translation/MicroJuvixToMiniC/Closure.hs Outdated Show resolved Hide resolved
src/Juvix/Translation/MicroJuvixToMiniC.hs Outdated Show resolved Hide resolved
@janmasrovira
Copy link
Collaborator

janmasrovira commented Jul 19, 2022 •
edited
Loading

@lukaszcz please reply to each of my comments if you have addressed them. It makes my life easier :)

@lukaszcz
Copy link
Collaborator Author

@lukaszcz please reply to each of my comments if you have addressed them. I makes my life easier :)

Okay, I just fixed those I didn't reply to.

janmasrovira
janmasrovira requested changes Jul 19, 2022
View reviewed changes
Copy link
Collaborator

@janmasrovira janmasrovira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor requests

src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
src/Juvix/Syntax/MicroJuvix/Language/Extra.hs Outdated Show resolved Hide resolved
@lukaszcz
Direct translation from MicroJuvix to MiniC.
44a908d 
* Closes issues #1341, #1338, #1339.
* Depends on PR [#8](anoma/juvix-stdlib#8) in juvix-stdlib.
* Translation/MonoJuvixToMiniC removed.
* MonoJuvix itself not removed because the MiniHaskell backend depends on it.
* Since MiniHaskell is deprecated, I see no point in wasting time on writing a translation from MicroJuvix to MiniHaskell.
@lukaszcz
Add tests for issues #1338 and #1339
cb3656c
@jonaprieto jonaprieto merged commit 65a44e0 into main Jul 19, 2022
@jonaprieto jonaprieto deleted the remove-monojuvix branch July 19, 2022 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@janmasrovira janmasrovira janmasrovira approved these changes

@paulcadman paulcadman Awaiting requested review from paulcadman

Assignees

@lukaszcz lukaszcz

Labels
enhancement New feature or request
Projects
None yet
Milestone
0.2.2
3 participants
@lukaszcz @janmasrovira @jonaprieto