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

disallow implicit return if result is used, don't use void context #24406

Draft
wants to merge 4 commits into
base: devel
Choose a base branch
from

Conversation

metagn
Copy link
Collaborator

@metagn metagn commented Nov 4, 2024

fixes #16855

Assigning to the result variable makes the assignment expression have a type of typed, which causes any subsequent expressions in the same statement list as the assignment to require a void type, so that the result variable cannot be overriden by implicit returns. This has multiple issues:

  1. This happens for any statement list containing an assignment to result, not just the proc body.
  2. Because this only triggers for direct assignments to the result variable, complex assignments like foo(result) can fool this and still allow an implicit return.

To fix this, we track if the result symbol is used, then error if an implicit return is attempted. An exception is granted to return statements which generate an assignment to the result variable: if the result variable was not used before, it will not count as used after.

@metagn
Copy link
Collaborator Author

metagn commented Nov 4, 2024

Bugs aside, mixing these is pretty common: https://github.com/arnetheduck/nim-results/blob/71d404b314479a6205bfd050f4fe5fe49cdafc69/results.nim#L1545-L1564

Something like varpartitions might be overkill, maybe a simpler version is to keep enforceVoidContext but opt it in to the expression context of the proc body with a TExprFlag, which I think makes sense but foo(result) wouldn't be fixed.

@metagn metagn marked this pull request as draft November 4, 2024 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

result variable not accepted in bool expression in parentheses
1 participant