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

Remove Intrinsic::Println #2115

Closed
vezenovm opened this issue Aug 1, 2023 · 3 comments · Fixed by #2358
Closed

Remove Intrinsic::Println #2115

vezenovm opened this issue Aug 1, 2023 · 3 comments · Fixed by #2358
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@vezenovm
Copy link
Contributor

vezenovm commented Aug 1, 2023

Problem

Now that we used Brillig foreign calls for printing to stdout we no longer need the ACIR based Println instruction as the builtin no longer exists in the stdlib. The Log directive can also then be removed from ACIR as other callers should also be using Brillig foreign calls to implement printing.

Happy Case

Remove Intrinsic::Println and any of its uses. It is also used for some of the SSA tests so these tests should be updated to use a new intrinsic such as ToBits or Sort.

Alternatives Considered

N/A - otherwise we are leaving dead code in the ACIR generation

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@vezenovm vezenovm added the enhancement New feature or request label Aug 1, 2023
@github-project-automation github-project-automation bot moved this to 📋 Backlog in Noir Aug 1, 2023
@Savio-Sou
Copy link
Collaborator

Just to confirm: would we lose any functionality with the removal, or is the new Brillig-based println already a superset of the old?

@vezenovm
Copy link
Contributor Author

vezenovm commented Aug 2, 2023

There is no longer a println builtin, the Brilig-based println has already superseded the previous method.

@vezenovm
Copy link
Contributor Author

@Ethan-000 If you have the capacity this could be a good issue to take on

@vezenovm vezenovm added the good first issue Good for newcomers label Aug 16, 2023
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants