Improve inference

[jakobnissen]

If one constructs a TranscodingStream and sets sharedbuf, the input must also be a TranscodingStream. The compiler now checks it. Previously, this would show up as a potential error in linters and such.

[Tokazama]

LGTM

[bicycle1885]

Sorry, I'm not sure what problem this pull request will fix. Can you share a case annoying you?

[jakobnissen]

It's a matter of inference and linting - behaviour is already correct. Here is a minimal example:

julia> using CodecZlib, JET

julia> @report_call GzipCompressorStream(IOBuffer())
═════ 1 possible error found ═════
┌ @ /home/jakob/.julia/packages/CodecZlib/ruMLE/src/compression.jl:48 CodecZlib.#GzipCompressorStream#2(Base.pairs(Core.NamedTuple()), #self#, stream)
│┌ @ /home/jakob/.julia/packages/CodecZlib/ruMLE/src/compression.jl:49 Core.kwfunc(CodecZlib.TranscodingStream)(Base.merge(Base.NamedTuple(), y), CodecZlib.TranscodingStream, _8, stream)
││┌ @ /home/jakob/.julia/packages/TranscodingStreams/MsN8d/src/stream.jl:118 TranscodingStreams.#TranscodingStream#5(bufsize, stop_on_end, sharedbuf, _3, codec, stream)
│││┌ @ /home/jakob/.julia/packages/TranscodingStreams/MsN8d/src/stream.jl:121 Base.getproperty(stream, :state)
││││┌ @ Base.jl:42 Base.getfield(x, f)
│││││ type IOBuffer has no field state
││││└──────────────
TranscodingStreams.TranscodingStream{GzipCompressor, IOBuffer}

As Julia linting and analysis gets better and better, this code part will keep showing up.

[jakobnissen]

Hm, although perhaps this inference will actually be solved by JuliaLang/julia#40880 - not sure!

[bicycle1885]

Thank you for explanation. In general, I don't like the idea of introducing annotation code for specific tools, especially if it is a false alarm. But if this change has large benefits to you and others, I'd say it's fine. Is this a must-have change, or a nice-to-have one?

[jakobnissen]

I completely get what you mean. It is not a good idea to change package X just so it works better with package Y. Because there can be a huge number of other packages that then may need changes to package X.

However, in this case, it's a bit different, I think, because it is really a kind of "type instability", or at least a mild inference issue. It's not JET.jl that thinks that the IOBuffer may have its state field accessed - it's the Julia compiler. However, I absolutely agree that it would be better if the compiler itself could figure out that the field will not be accessed if the stream is not a TranscodingStream. Then we wouldn't need to help it.

The problem is that, as far as I can tell, the value of sharedbuf does not constant fold to false. It would also be better if we could somehow make that constant fold, but in the absence of both a sufficiently smart compiler, and sharedbuf constant folding, I think it's best to help the compiler along to get better type inference.

[bicycle1885]

OK, then, let's annotate the code. But before merging, can you add a comment that refers to this pull request so that we don't miss the reason why the annotation is introduced. Git blame is not reliable for that purpose IMO.