Skip to content

fix fallback code path in take!(::IOBuffer) method #58762

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

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

aviatesk
Copy link
Member

JET told me that the data local variable was inparticular is undefined at this point.
After reviewing this code, I think this code path is unreachable actually since bytesavailable(io::IOBuffer) returns 0 when io has been closed. So it's probably better to make it clear.

@aviatesk aviatesk requested a review from vtjnash June 17, 2025 16:03
@jakobnissen
Copy link
Member

You are right - this branch is unreachable, but it has nothing to do with close IIUC. The only API to create a non-seekable IOBuffer is with the PipeBuffer constructor, and those are always writeable. So you can't have a non-seekable, non-writeable GenericIOBuffer.

@nsajko nsajko added the io Involving the I/O subsystem: libuv, read, write, etc. label Jun 17, 2025
@aviatesk
Copy link
Member Author

Thanks for explaining, @jakobnissen.
So it looks like this code path should raise an explicit error. Do you think the current error message is okay, or would something like "Invalid API usage of IOBuffer" be better?

@aviatesk aviatesk added the backport 1.12 Change should be backported to release-1.12 label Jun 18, 2025
@jakobnissen
Copy link
Member

I think your error message is fine. One would have to use a private, undocumented constructor to hit it, and then all kinds of things can go wrong

JET told me that the `data` local variable was inparticular is
undefined at this point.
After reviewing this code, I think this code path is unreachable
actually since `bytesavailable(io::IOBuffer)` returns `0` when
`io` has been closed. So it's probably better to make it clear.
@aviatesk aviatesk force-pushed the avi/fix-IOBuffer-take! branch from e90797f to 5535a13 Compare June 18, 2025 08:59
@aviatesk aviatesk merged commit 2fbf5d8 into master Jun 18, 2025
4 of 7 checks passed
@aviatesk aviatesk deleted the avi/fix-IOBuffer-take! branch June 18, 2025 11:39
aviatesk added a commit that referenced this pull request Jun 18, 2025
JET told me that the `data` local variable was inparticular is undefined
at this point.
After reviewing this code, I think this code path is unreachable
actually since `bytesavailable(io::IOBuffer)` returns `0` when `io` has
been closed. So it's probably better to make it clear.
@aviatesk aviatesk mentioned this pull request Jun 18, 2025
60 tasks
@aviatesk aviatesk removed the backport 1.12 Change should be backported to release-1.12 label Jun 18, 2025
aviatesk added a commit that referenced this pull request Jun 19, 2025
JET's new analysis pass now detects local variables that may be
undefined, which has revealed such issues in several functions within
Base (#58762).

This commit addresses local variables whose definedness the compiler
cannot properly determine, primarily in functions reachable from JET's
test suite. No functional changes are made.
aviatesk added a commit that referenced this pull request Jun 19, 2025
JET's new analysis pass now detects local variables that may be
undefined, which has revealed such issues in several functions within
Base (#58762).

This commit addresses local variables whose definedness the compiler
cannot properly determine, primarily in functions reachable from JET's
test suite. No functional changes are made.
aviatesk added a commit that referenced this pull request Jun 19, 2025
JET's new analysis pass now detects local variables that may be
undefined, which has revealed such issues in several functions within
Base (#58762).

This commit addresses local variables whose definedness the compiler
cannot properly determine, primarily in functions reachable from JET's
test suite. No functional changes are made.
aviatesk added a commit that referenced this pull request Jun 20, 2025
JET's new analysis pass now detects local variables that may be
undefined, which has revealed such issues in several functions within
Base (#58762).

This commit addresses local variables whose definedness the compiler
cannot properly determine, primarily in functions reachable from JET's
test suite. No functional changes are made.
aviatesk added a commit that referenced this pull request Jun 20, 2025
JET's new analysis pass now detects local variables that may be
undefined, which has revealed such issues in several functions within
Base (#58762).

This commit addresses local variables whose definedness the compiler
cannot properly determine, primarily in functions reachable from JET's
test suite. No functional changes are made.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
io Involving the I/O subsystem: libuv, read, write, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants