crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.35k stars 1.62k forks source link

`.as?` doesn't behave like `.as` when casting reference to pointer #14491

Open ysbaddaden opened 5 months ago

ysbaddaden commented 5 months ago

I noticed Reference#as?(Void*) doesn't behave like Reference#as(Void*). For example:

class Foo; end

foo = Foo.new
p foo.as(Void*)  # => Pointer(Void)@x7fd2152b9a00
p foo.as?(Void*) # => nil

I believe this is an error in .as?. The only difference between these two pseudo methods should be in error handling (raise vs. nil), and casting a reference to a pointer is an acceptable behavior (a reference is a pointer).

straight-shoota commented 5 months ago

I have some vague recollection that this had been mentioned before. But as is not great on searchability 🙈

ysbaddaden commented 5 months ago

We talked about it in on slack a while back :speak_no_evil:

The pseudo methods are implemented as the Cast and NilableCast AST nodes in src/compiler/crystal/semantic/bindings.cr. The implementation is identical (calls for a refactor) except that Cast explicitly checks for pointers while NilableCast doesn't (eventually the methods differ to handle a nil type in NilableCast):

https://github.com/crystal-lang/crystal/blob/944bee24a2ad34160b3f9a4eadaf673c53d4e91f/src/compiler/crystal/semantic/bindings.cr#L420

https://github.com/crystal-lang/crystal/blob/944bee24a2ad34160b3f9a4eadaf673c53d4e91f/src/compiler/crystal/semantic/bindings.cr#L477

ysbaddaden commented 5 months ago

Nope, that doesn't fix anything: the type is correctly inferred as Pointer(Void) | Nil. The issue is more likely to be in the codegen where Cast takes cares of pointers while NilableCast doesn't.

https://github.com/crystal-lang/crystal/blob/944bee24a2ad34160b3f9a4eadaf673c53d4e91f/src/compiler/crystal/codegen/codegen.cr#L1401

ysbaddaden commented 5 months ago

Hum, this is worst than just references: .as? fails with pointers, unless the pointer type is identical:

foo = Pointer(Int32).new(0xdeadbeef)
p foo.as(Int32*)  # => Pointer(Void)@xdeadbeef
p foo.as?(Int32*) # => Pointer(Void)@xdeadbeef
foo = Pointer(Int32).new(0xdeadbeef)
p foo.as(Void*)  # => Pointer(Void)@xdeadbeef
p foo.as?(Void*) # => nil

I'm pretty sure the codegen linked above is the culprit: it doesn't account for pointers at all. That being said, I'm not sure how to properly fix it...

straight-shoota commented 5 months ago

The behaviour is identical in the interpreter. So if it's a codegen issue, the interpreter has the same implementation.

ysbaddaden commented 5 months ago

Confirmed: the filtered_type variable is nil because we can't filter Pointer(Void) from Pointer(Int32) for example, which the compiler interprets as "impossible cast" and always codegens a nil.

ysbaddaden commented 5 months ago

The interpreter behaves just like the LLVM codegen: it immediately returns nil when filtered_type doesn't match.

straight-shoota commented 4 months ago

Related: https://github.com/crystal-lang/crystal/issues/11058