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 out parameter from pony_os_stdin_read #4000

Merged
merged 1 commit into from
Feb 6, 2022

Conversation

jemc
Copy link
Member

@jemc jemc commented Feb 6, 2022

The signature of the pony_os_stdin_read function has been simplified to remove the bool* out_again parameter.

-PONY_API size_t pony_os_stdin_read(char* buffer, size_t space, bool* out_again)
+PONY_API size_t pony_os_stdin_read(char* buffer, size_t space)

It is permitted to call the pony_os_stdin_read function again in a loop if the return value is greater than zero, and the platform is not windows. Given that those two conditions are enough information to make a decision, the out_again parameter is not needed, and can be removed.

Technically this is a breaking change, because the function is prefixed with pony_ and is thus a public API. But it is unlikely that any code out there is directly using the pony_os_stdin_read function, apart from the Stdin actor in the standard library, which has been updated in its internal implementation details to match the new signature.

The signature of the `pony_os_stdin_read` function has been simplified to remove the `bool* out_again` parameter.

```diff
-PONY_API size_t pony_os_stdin_read(char* buffer, size_t space, bool* out_again)
+PONY_API size_t pony_os_stdin_read(char* buffer, size_t space)
```

It is permitted to call the `pony_os_stdin_read` function again in a loop if the return value is greater than zero, and the platform is not windows. Given that those two conditions are enough information to make a decision, the `out_again` parameter is not needed, and can be removed.

Technically this is a breaking change, because the function is prefixed with `pony_` and is thus a public API. But it is unlikely that any code out there is directly using the `pony_os_stdin_read` function, apart from the `Stdin` actor in the standard library, which has been updated in its internal implementation details to match the new signature.
@jemc jemc requested a review from a team February 6, 2022 20:32
@jemc jemc self-assigned this Feb 6, 2022
@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Feb 6, 2022
@jemc jemc added changelog - changed Automatically add "Changed" CHANGELOG entry on merge discuss during sync Should be discussed during an upcoming sync and removed discuss during sync Should be discussed during an upcoming sync labels Feb 6, 2022
@jemc jemc merged commit f4d8fe2 into ponylang:main Feb 6, 2022
@jemc jemc deleted the simplify-stdin-read branch February 6, 2022 23:09
github-actions bot pushed a commit that referenced this pull request Feb 6, 2022
github-actions bot pushed a commit that referenced this pull request Feb 6, 2022
jemc added a commit to savi-lang/runtime-bitcode that referenced this pull request Feb 6, 2022
This includes a mainstream acceptanc eof the objectmap patch (now removed here)
as well as a simplification of the stdin reading C API (see ponylang/ponyc#4000)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants