-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Lean: Change loops to use native lean loops #1086
Lean: Change loops to use native lean loops #1086
Conversation
Test Results 13 files 29 suites 0s ⏱️ Results for commit d793b64. ♻️ This comment has been updated with latest results. |
9d7a5e7
to
1485f95
Compare
Do you also plan on using |
TODO: handle while and remove useless loop_ functions
1485f95
to
255a573
Compare
test/lean/loop.expected.lean
Outdated
@@ -128,17 +128,22 @@ def concat_str_dec (str : String) (x : Int) : String := | |||
(HAppend.hAppend str (Int.repr x)) | |||
|
|||
/-- Type quantifiers: n : Nat, m : Nat, 0 ≤ m, 0 ≤ n -/ | |||
def foreach_loop (m : Nat) (n : Nat) : Nat := | |||
def foreach_loop (m : Nat) (n : Nat) : SailM Nat := do |
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.
Why does this PR introduce effects here?
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.
This was a mistake, it was fixed in the next commit
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 still see effects here.
test/lean/loop.expected.lean
Outdated
@@ -128,17 +128,22 @@ def concat_str_dec (str : String) (x : Int) : String := | |||
(HAppend.hAppend str (Int.repr x)) | |||
|
|||
/-- Type quantifiers: n : Nat, m : Nat, 0 ≤ m, 0 ≤ n -/ | |||
def foreach_loop (m : Nat) (n : Nat) : Nat := | |||
def foreach_loop (m : Nat) (n : Nat) : SailM Nat := do |
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 still see effects here.
I haven't updated the tests yet because there are some issues that need #1089 to be merged first |
This will fix the issues with #1071
To be merged after #1084