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 BuiltinContext and Options arguments from functions #945

Closed
brianhuffman opened this issue Dec 3, 2020 · 0 comments · Fixed by #949
Closed

Remove BuiltinContext and Options arguments from functions #945

brianhuffman opened this issue Dec 3, 2020 · 0 comments · Fixed by #949
Labels
tech debt Issues that document or involve technical debt

Comments

@brianhuffman
Copy link
Contributor

Most of the haskell functions that implement saw-script primitives take two extra arguments of types BuiltinContext and Options. Some of the functions don't use these arguments at all. But none of the functions should need to use them, since all of the saw-script primitives are implemented in the TopLevel monad (or another monad layered on top of it) and the same information should be obtainable from the monad state.

Passing all these extra arguments around makes the code more complicated, harder to read, and more error prone (as it introduces the possibility of passing arguments that don't match the state in the monad). We should get rid of all of them.

@brianhuffman brianhuffman added the tech debt Issues that document or involve technical debt label Dec 3, 2020
brianhuffman pushed a commit that referenced this issue Dec 3, 2020
Removing those parameters also necessitated the addition of
a `roBasicSS` field to the `TopLevelRO` datatype.

Fixes #945.
brianhuffman pushed a commit that referenced this issue Dec 4, 2020
Removing those parameters also necessitated the addition of
a `roBasicSS` field to the `TopLevelRO` datatype.

Fixes #945.
brianhuffman pushed a commit that referenced this issue Dec 4, 2020
Removing those parameters also necessitated the addition of
a `roBasicSS` field to the `TopLevelRO` datatype.

Fixes #945.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech debt Issues that document or involve technical debt
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant