IO.write is broken in 0.35.0 #9454
Labels
Comments
jhass
added
kind:bug
|
@jhass Update, seems like basically most IO.write are broken, can't see how this passed CI class IO::Hexdump < IO
def write(buf : Bytes) : Int64?
return 0i64 if buf.empty?
@io.write(buf).tap do
@output.puts buf.hexdump if @write
end
end
end
class IO::Stapled < IO
# Writes a slice to `writer`.
def write(slice : Bytes) : Int64?
check_open
return 0i64 if slice.empty?
@writer.write(slice)
end
end
module OpenSSL
class DigestIO < IO
def write(slice : Bytes) : Int64?
return 0i64 if slice.empty?
if @mode.write?
digest_algorithm.update(slice)
end
io.write(slice)
end
end
endetc... and many more |
bararchy
changed the title
|
Any code to reproduce this? |
|
@asterite seems that |
|
@bararchy That works fine for me, no warnings |
|
Are you compiling just that program without any dependencies?
Do you have other IOs in your program? |
asterite
added
kind:question
and removed
kind:bug
|
@asterite it's being called from formatter = ::Log::Formatter.new do |entry, io|
io << "[#{Time.utc.to_rfc3339}] [#{entry.severity}] #{entry.message}"
end |
|
I don't have anythings else, and the IO is basically formatter = ::Log::Formatter.new do |entry, io|
io << "[#{Time.utc.to_rfc3339}] [#{entry.severity}] #{entry.message}"
end
backend = ::Log::IOBackend.new
backend.formatter = formatterSo I guess it's |
|
I'm trying all of your snippets, no warnings whatsoever. Could you post a complete snippet? The snippet should be compilable by doing Can you point to a github repo? Are you sure it's just a single file without any other dependencies? |
|
Maybe also record what you are doing so we can see what's going on? Print |
|
@asterite it's from our main project, a large, large crystal program :) Full error trace |
|
I ran into this too. It happened to us on 0.35, and ended up being because some of our custom IO subtypes did not have the updated IO contract of |
|
@bararchy Try searching |
|
This is what I was able to come up with: require "log"
puts(Time.utc)
class Foo < IO
def read(slice : Bytes)
raise IO::Error.new("read-only")
end
def write(slice : Bytes) : Nil
Log.info { "test" }
end
endFixing the IO contract in |
|
It's a shame we dont just get the path the the IO which actually has the wrong def in it . Thanks guys, im on it. |
|
Yes, I already noticed that with Isolated example: module Io
abstract def write : Int64
end
class Foo
include Io
def write : Int64 # Error: method must return Int64 but it is returning Nil
Bar.new.write
end
end
class Bar
include Io
def write # The error should be reported here!
nil
end
end
Foo.new.write |
|
Well, the abstract def checking is done before the semantic code. The problem is that for some reason warnings are delayed and shown until much later, I don't know why 🤷 |
|
Found them 😢
|
|
I don't think this should be considered as fixed. The compiler error is completely confusing and needs to be improved. It should point to the location that actually needs attention. |
|
One aspect of the problem is that write is not an abstract method. Without the notion of redefine/override there is no way to express that the protocol/interface should be enforced. I thought that since abstract method are enforced, then maybe having an But, that wont do it. The following example unfortunately compiles abstract class Base
abstract def m : Int64
end
class A < Base
end
class A
def m : Int64
0i64
end
end
class B < A
def m
nil
end
end
a = A.new || B.new
pp! typeof(a.m) # => (Int64 | Nil) |
|
If the original concern of this issue is fixed, let's open another one to improve the error message situation |
|
@bcardiff But Line 103 in d08b646 That's why this is so weird. Most use cases will probably be fixed if the abstract method return type is properly enforced on the wrapped method. See my isolated example in #9454 (comment) |
|
Just to be clear, I already explained what the problem is in a previous comment: #9454 (comment) |
|
Created a new issue to track the confusing compiler error in #9460 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Error
The text was updated successfully, but these errors were encountered: