Make Formatted <: IO to avoid invalidations

[serenity4]

Addresses #396, and further seems to simplify the implementation a little.

This change is a bit weird, as Formatted itself does not semantically represent an IO type, it is only Stream that does. But since Stream can only subtype a single abstract type, that is the only way to make it Stream <: ... <: IO such that the type hierarchy is preserved.

The potential drawback I see is that it probably causes more specialization, threading Stream through IO functions. At first I attempted to keep the more specific read/write methods for Stream, but because Stream is now an IO ambiguity errors pop up which require more methods to be defined manually to be resolved, at which point I thought having a minimal IO implementation might be better. I can try to maximally avoid specialization by adding these methods back, but it is quite likely that other ambiguity errors pop up in the future if going down that path. I believe that the invalidation fixes outweigh the added specialization.

I'm happy to discuss/drop this PR if the subtyping change it not desired, in which case we can try to come up with another solution to #396 (if it exists). I'm also curious to know if there was some discussion about having Stream <: IO, seeing that it implements lots of IO interface functions.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.22%. Comparing base (4c93073) to head (82fb2f0).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
+ Coverage   91.71%   92.22%   +0.50%     
==========================================
  Files          10       10              
  Lines         712      707       -5     
==========================================
- Hits          653      652       -1     
+ Misses         59       55       -4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[nhz2]

I agree that this change is strange because it makes File an IO. I think the solution to #396 would be to have AnnotatedString behave like a regular AbstractString if it is a subtype of AbstractString.

[serenity4]

The discussion at KristofferC/OhMyREPL.jl#355 makes me believe that it isn't really possible, unfortunately (see notably this comment).

[nhz2]

My understanding of that comment makes me think that there are a few options to improve things:

1. Don't have AnnotatedString subtype AbstractString.
2. Have AnnotatedString subtype AbstractString, but existing AbstractString functions should ignore annotations.

[serenity4]

I believe in theory these were both considered, as explained in the answer to that comment: KristofferC/OhMyREPL.jl#355 (comment). In practice, it seems like the slightly less intuitive/ideal design brings good trade-offs, with less code churn and desirable behavior, one downside being invalidations popping up. Ideally it's not downstream code that should deal with this, but I'm exploring whether we could alleviate this deliberate downside with as noticeable of an impact here.

[serenity4]

As Stream fundamentally represents some IO-like type, would it be possible to slightly change the design such that Formatted is more of a trait, and does not require to bite the subtyping silver bullet? This way, we could have Stream subtype IO directly. What do you think?

[nhz2]

There is already a stream(s::Stream) function to get an IO representation of a Stream, so I'm not sure subtyping IO is needed.

[serenity4]

The idea is to address #396; subtyping IO is one solution (and the only one I know of so far). At the same time, we can see in this PR that it brings a few simplifications to the code. The question is not whether this is needed, but whether this is acceptable or even desirable given these arguments.

[nhz2]

This should be fixed by JuliaLang/julia#55593

I think it might make sense for Stream to be an IO, but I don't think File should be an IO

Also making Stream an IO will require implementing or forwarding most of the IO methods. It is not enough to just forward read and write for single bytes.

[serenity4]

Nice, thanks!