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

ftp: replace size/1 by XXX_size/1 #6793

Conversation

kikofernandez
Copy link
Contributor

The size/1 BIF is not optimized by the JIT, and its use can result in worse types for Dialyzer.

When one knows that the value being tested must be a tuple, tuple_size/1 should always be preferred.

When one knows that the value being tested must be a binary, byte_size/1 should be preferred. However, byte_size/1 also accepts a bitstring (rounding up size to a whole number of bytes), so one must make sure that the call to byte_size/ is preceded by a call to is_binary/1 to ensure that bitstrings are rejected. Note that the compiler removes redundant calls to is_binary/1, so if one is not sure whether previous code had made sure that the argument is a binary, it does not harm to add an is_binary/1 test immediately before the call to byte_size/1.

This PR extends #6770 simply in the branch naming convention. No changes in the implementation

The <c>size/1</c> BIF is not optimized by the JIT, and its use can
result in worse types for Dialyzer.

When one knows that the value being tested must be a tuple,
<c>tuple_size/1</c> should always be preferred.

When one knows that the value being tested must be a binary,
<c>byte_size/1</c> should be preferred. However, <c>byte_size/1</c> also
accepts a bitstring (rounding up size to a whole number of bytes), so
one must make sure that the call to <c>byte_size/</c> is preceded by a
call to <c>is_binary/1</c> to ensure that bitstrings are rejected. Note
that the compiler removes redundant calls to <c>is_binary/1</c>, so if
one is not sure whether previous code had made sure that the argument is
a binary, it does not harm to add an <c>is_binary/1</c> test immediately
before the call to <c>byte_size/1</c>.
@github-actions
Copy link
Contributor

github-actions bot commented Feb 6, 2023

CT Test Results

Tests are running... https://github.com/erlang/otp/actions/runs/4101317794

Results for commit c028580

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

  • No CT logs found
  • No HTML docs found
  • No Windows Installer found

// Erlang/OTP Github Action Bot

@kikofernandez kikofernandez merged commit 8b7906c into erlang:maint Feb 6, 2023
@kikofernandez kikofernandez deleted the kiko/ftp/replace-size-by-xxx_size/GH-6672/OTP-18432 branch February 6, 2023 08:44
kikofernandez added a commit to kikofernandez/otp that referenced this pull request Feb 9, 2023
this commit fixes a mistake in erlangGH-6793 where a guard was removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:PS Assigned to OTP team PS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant