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

interp: default to None for non-valued statements #1079

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

firelizzard18
Copy link
Contributor

@firelizzard18 firelizzard18 commented Apr 14, 2021

Here am using 'None' to refer to the functional programming concept of an empty result. I added interp.EmptyResult since that is far clearer than 'None', especially to users not familiar with functional programming jargon.

Updates *Interpreter.ast to use val: EmptyResult as the default, instead of new(interface{}). Updates genValue to always return node.val for a block statement where the last statement is var x = 1, x := 1, or x = 1.

As a result, variable declarations and assignments return a value of interp.EmptyResult.

@mvertes
Copy link
Member

mvertes commented May 10, 2021

The current state is not consistent. It would be better to always disable result output for assignment rather than adding an option, for consistency and simplicity. Could you modify your PR in that direction ?
Thanks

Update AST to use EmptyResult instead of new(interface{}) as the default
val for a node. Update genValue to always return node.val for variable
declaration, definition, and assignment.
@firelizzard18 firelizzard18 changed the title Add an option to suppress output from assignments interp: default to None for non-valued statements Aug 23, 2021
@firelizzard18 firelizzard18 marked this pull request as ready for review August 23, 2021 21:50
@firelizzard18
Copy link
Contributor Author

@mvertes This is ready for review. Now, all variable declarations and assignments return the special value interp.EmptyResult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants