Use timer:time for non-negative timeout values #9574
Conversation
As suggested in erlang#9515. In some parts of the code, `pos_integer()` is currently used for timeouts, which can't be replaced with `timer:time()` as it is specified as `integer() >= 0`. Locally tested by running dialyzer with updated preloaded code.
CT Test Results 12 files 268 suites 4h 29m 38s ⏱️ For more details on these failures, see this check. Results for commit 07f6622. ♻️ 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 |
| @@ -198,7 +198,7 @@ All TCP socket options are accepted except `active`, `binary`, `deliver`, | |||
| Options :: | |||
| [{port, integer()} | | |||
| {log, function()} | | |||
| {timeout, integer()} | | |||
There was a problem hiding this comment.
We consider this a documenation bug, should be timeout()
| millisec_passed(T0) -> | ||
| %% OTP 18 |
| @@ -290,7 +290,7 @@ way, option `send_timeout` comes in handy. | |||
| {reuseaddr, boolean()} | | |||
| {reuseport, boolean()} | | |||
| {reuseport_lb, boolean()} | | |||
| {send_timeout, non_neg_integer() | infinity} | | |||
There was a problem hiding this comment.
We think timeout() should be used
| @@ -47,7 +47,7 @@ | |||
| -define(DETS_CALL(Pid, Req), {'$dets_call', Pid, Req}). | |||
|
|
|||
| -type access() :: 'read' | 'read_write'. | |||
| -type auto_save() :: 'infinity' | non_neg_integer(). | |||
There was a problem hiding this comment.
We think timeout() should be used
| @@ -311,7 +311,7 @@ corresponding value can be of any type.[](){: #ci_control_pid } | |||
| The timeout time is in milliseconds. A value of 0 (zero) means that the proxy | |||
| process will exit directly after the reply has been delivered. | |||
|
|
|||
| Value type: [non_neg_integer()](`t:erlang:non_neg_integer/0`) | |||
| @@ -259,7 +259,7 @@ _Options:_ | |||
| {overload_check, {MSec, Module, Function}} | {flush, MSec} | | |||
| resume | {resume, MSec} | {queue_size, non_neg_integer()}, | |||
| TimerSpec :: MSec | {MSec, stop_opts()}, | |||
| MSec :: integer(), | |||
| MSec :: timer:time(), | |||
| Module :: atom(), | |||
There was a problem hiding this comment.
Probably ok, anyone think differently ?
| @@ -408,7 +408,7 @@ corresponding value can be of any type.[](){: #ci_control_pid } | |||
|
|
|||
| See also [transaction sender](megaco_run.md#transaction_sender) for more info. | |||
|
|
|||
| Value type: [non_neg_integer()](`t:erlang:non_neg_integer/0`) | |||
| @@ -170,7 +170,7 @@ get_disk_info(Path) -> | |||
| -doc """ | |||
| Returns the time interval, in milliseconds, for the periodic disk space check. | |||
| """. | |||
| -spec get_check_interval() -> Milliseconds :: integer(). | |||
There was a problem hiding this comment.
Probably ok, anyone think differently ?
| @@ -246,7 +246,7 @@ get_system_memory_data() -> | |||
| -doc """ | |||
| Returns the time interval, in milliseconds, for the periodic memory check. | |||
| """. | |||
| -spec get_check_interval() -> Milliseconds :: integer(). | |||
There was a problem hiding this comment.
Probably ok, anyone think differently ?
IngelaAndin
requested review from
dgud,
garazdawi,
rickard-green and
RaimoNiskanen
|
@jchristgit This became quite hard to review as it changes so many different applications at once. Although the changes are similar in one way there are quite a few things to consider. And that is why I add quite a few reviewers for this so maybe wait a little before deciding how to proceed. |
|
@IngelaAndin thanks for the review. Sorry, I hope I don't cause too much trouble! I've pushed a second commit that addresses your comments where the outcome was clear. I didn't update the existing commit in case we want to change something there, will squash when everything looks good. |
As suggested in #9515. In some parts of the code,
pos_integer()is currently used for timeouts, which can't be replaced withtimer:time()as it is specified asinteger() >= 0.Locally tested by running dialyzer with updated preloaded code.