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.

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

Extend concrete evaluation #101

Merged
merged 34 commits into from
Jun 21, 2024
Merged

Extend concrete evaluation #101

merged 34 commits into from
Jun 21, 2024

Conversation

joaomhmpereira
Copy link
Member

@joaomhmpereira joaomhmpereira linked an issue Mar 28, 2024 that may be closed by this pull request
@filipeom
Copy link
Member

filipeom commented Apr 1, 2024

Hey João, sorry for the massive delay. However, in an effort to reduce your workload, I've updated issue #99. I believe we've completed most of the operators, but please pay special attention to the ones annotated with ❗, as I'm not sure if we've included them yet.

In this PR, you can remove the float operators you added. I've begun moving them directly to ecma-sl here: formalsec/ECMA-SL#stdlib

Regarding the string operators you included, I'd like to keep them, excluding the lowercase/uppercase functions. We'll also transfer those to ecma-sl's standard library.

Copy link
Member

@filipeom filipeom left a comment

Choose a reason for hiding this comment

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

By the way, could you add simple tests to test_{unop|binop|triop|cvtop}, similar to the ones already present, just to verify if the construction of the operators you added is being correctly simplified in practice?

I don't want to rush this. I'm going to pin this branch to https://github.com/formalsec/ECMA-SL/tree/stdlib and see if we can start the value migration

@joaomhmpereira
Copy link
Member Author

@filipeom I think this is finally ready for review

@filipeom
Copy link
Member

Sorry for the delay, I did a quick check of the code, I'll try to give more feedback as I start to get more time

@filipeom
Copy link
Member

filipeom commented Jun 18, 2024

There are a few more changes I want to add here before we merge. @joaomhmpereira you merged main into here on your last commit fe136d2?

@joaomhmpereira
Copy link
Member Author

joaomhmpereira commented Jun 18, 2024

There are a few more changes I want to add here before we merge. @joaomhmpereira you merged main into here on your last commit fe136d2?

Yes, but it was an older version where the tests for the solvers were not fixed yet

@filipeom
Copy link
Member

Yes, but it was an older version where the tests for the solvers were not fixed yet

Okok, so I'll rebase it on main to fix conflicts as well

@filipeom filipeom force-pushed the extend_ops branch 2 times, most recently from a83ef4f to b1c2116 Compare June 21, 2024 09:05
@filipeom
Copy link
Member

Don't know why the CI is failing on bitwuzla. But it should be ok

@filipeom filipeom merged commit 42c2eb3 into main Jun 21, 2024
9 of 10 checks passed
@filipeom filipeom deleted the extend_ops branch June 21, 2024 12:47
@filipeom filipeom mentioned this pull request Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants