-
Notifications
You must be signed in to change notification settings - Fork 98
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
Allow sending messages to registered process with !
#1047
Conversation
7e74036
to
447795f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a few comments, anyway I think we should really add a test case.
src/libAtomVM/opcodesswitch.h
Outdated
@@ -2213,7 +2213,16 @@ HOT_FUNC int scheduler_entry_point(GlobalContext *glb) | |||
#endif | |||
|
|||
#ifdef IMPL_EXECUTE_LOOP | |||
int local_process_id = term_to_local_process_id(x_regs[0]); | |||
term process_term = x_regs[0]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would call it just recipient
or with any name that is not biased towards a process.
src/libAtomVM/opcodesswitch.h
Outdated
int local_process_id = term_to_local_process_id(x_regs[0]); | ||
term process_term = x_regs[0]; | ||
int local_process_id; | ||
if (term_is_atom(process_term)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should check if recipient
is a pid
first, after that if it is an atom and registered, and as last thing just raise badarg.
447795f
to
46e4b80
Compare
aa82dfc
to
bb49051
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
src/libAtomVM/opcodesswitch.h
Outdated
local_process_id = term_to_local_process_id(recipient_term); | ||
} else if (term_is_atom(recipient_term)) { | ||
local_process_id = globalcontext_get_registered_process(ctx->global, term_to_atom_index(recipient_term)); | ||
if (UNLIKELY(local_process_id == 0 )) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
leftover space here: if (UNLIKELY(local_process_id == 0 )) {
-> if (UNLIKELY(local_process_id == 0)) {
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
CHANGELOG.md
Outdated
@@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 | |||
### Fixed | |||
|
|||
- ESP32: fix i2c_driver_acquire and i2c_driver_release functions, that were working only once. | |||
- Sending messages to registered processes using the `!` operator now works. | |||
- Fixed bug in `OP_SEND` that would accept sending a message to any integer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Te situation was a bit worse: we did allow send to any term without raising any error, and for integers was even working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the explanation of the bug.
Enables sending messages to registered processes using the shorhand `!` operator. If a message is sent to an atom name that is not registered using the `!` operator a `badarg` run-time error occurs, matching OTP behaviour. closes atomvm#1011 closes atomvm#98 Signed-off-by: Winford <winford@object.stream>
bb49051
to
eab4e32
Compare
Enables sending messages to registered processes using the shorthand
!
operator. If a message is sent to an atom name that is not registered using the!
operator abadarg
run-time error occurs, matching OTP behaviour.Closes #1011
Closes #98
These changes are made under both the "Apache 2.0" and the "GNU Lesser General
Public License 2.1 or later" license terms (dual license).
SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later