Skip to content

Conversation

@Maria-12648430
Copy link
Contributor

This way of checking for monitor or {monitor, ...} in the options lists seems to me much simpler than the current nested case.

There is a slight (but IMO inconsequential) change in behavior: if there is a {monitor, ...} tuple in the option list, the current code will result in a call error(badarg, [M, F, A, T, Opts]) if it is a 2-tuple, but fail with a function_clause error if it is not a 2-tuple; with my changes, any {monitor, ...} tuple will result in a call error(badarg, [M, F, A, T, Opts]).

@github-actions
Copy link
Contributor

github-actions bot commented Apr 22, 2025

CT Test Results

    2 files     97 suites   1h 8m 56s ⏱️
2 207 tests 2 154 ✅ 52 💤 1 ❌
2 586 runs  2 528 ✅ 57 💤 1 ❌

For more details on these failures, see this check.

Results for commit 8d872c3.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@Maria-12648430
Copy link
Contributor Author

On a side note, the type proc_lib:start_spawn_option/0 does not list the {async_dist, Enabled :: boolean()} option (see https://www.erlang.org/doc/apps/erts/erlang#t:spawn_opt_option/0). Should it?

@essen
Copy link
Contributor

essen commented Apr 22, 2025

@Maria-12648430
Copy link
Contributor Author

Maria-12648430 commented Apr 22, 2025

Can't this use https://www.erlang.org/doc/apps/stdlib/proplists.html#is_defined/2 ?

Depends...

As far as the result goes, yes.

OTOH for one, lists:member and lists:keymember are NIFs, but proplists:is_defined is not. But proplists:is_defined does a single pass through the list, lists:member+lists:keymember does two. This may make not much of a difference whichever way, lists of spawn opts are unlikely to be very long.

OTOH for another, lists of spawn options are not proplists; in a proplist, the first option in the list "wins" if there are multiple with the same key, in a spawn options list this is not defined (ATM, it seems to be the last). This is just an academic objection, of course. For practical purposes, we could of course use proplists:is_defined. In theory however, it just so happens to do the job.

Anyway, I have no strong opinion one way or the other 🤷‍♀️

@rickard-green rickard-green added the team:VM Assigned to OTP team VM label Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:VM Assigned to OTP team VM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants