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

More improvements to inference in IO #36449

Merged
merged 4 commits into from
Jul 13, 2020
Merged

More improvements to inference in IO #36449

merged 4 commits into from
Jul 13, 2020

Conversation

timholy
Copy link
Sponsor Member

@timholy timholy commented Jun 26, 2020

Also continues the work in #36280.

@@ -289,7 +289,7 @@ function _unique!(f, A::AbstractVector, seen::Set, current::Integer, i::Integer)
end
i += 1
end
return resize!(A, current - firstindex(A) + 1)
return resize!(A, current - firstindex(A) + 1)::typeof(A)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been wondering if we should separate "call API" and "overload API." It makes much easier to imposing an invariance like this one. For example, we can have

function resize!(a, n)
    __resize!__(a, n)
    return a::typeof(a)
end

It asserts the return type and also makes sure that the object a is returned. This is useful for something like push! where some implementations forget to return the correct object #34274.

readuntil(io::AbstractPipe, arg::AbstractVector; kw...) = readuntil(pipe_reader(io), arg; kw...)
readuntil_vector!(io::AbstractPipe, target::AbstractVector, keep::Bool, out) = readuntil_vector!(pipe_reader(io), target, keep, out)
readbytes!(io::AbstractPipe, target::AbstractVector{UInt8}, n=length(target)) = readbytes!(pipe_reader(io), target, n)
write(io::AbstractPipe, byte::UInt8) = write(pipe_writer(io)::IO, byte)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to define _pipe_writer(io) = pipe_writer(io)::IO?

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea, but annotating each call site seems reasonable too. The _pipe_writer solution would fall apart if people start defining new _pipe_writer methods ☹️. Sometimes it might be nice to declare a function to be non-specializable!

Copy link
Sponsor Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to adopt that strategy in #36453, since there were so many calls to buffer. Here, however, I could imagine there being external extensions of pipe_writer.

@timholy timholy added the compiler:latency Compiler latency label Jun 27, 2020
base/set.jl Outdated Show resolved Hide resolved
base/io.jl Outdated Show resolved Hide resolved
@timholy
Copy link
Sponsor Member Author

timholy commented Jul 8, 2020

I'll merge soon barring any further concerns.

@timholy timholy merged commit b0a55b8 into master Jul 13, 2020
@timholy timholy deleted the teh/more_io_inference branch July 13, 2020 10:22
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:latency Compiler latency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants