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

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

Open
ysbaddaden opened this issue Apr 15, 2024 · 8 comments · May be fixed by #14499
Open

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

ysbaddaden opened this issue Apr 15, 2024 · 8 comments · May be fixed by #14499

Comments

@ysbaddaden
Copy link
Contributor

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
Copy link
Member

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

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Apr 15, 2024

We talked about it in on slack a while back 🙊

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):

if obj_type && !(obj_type.pointer? || to_type.pointer?)

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Apr 15, 2024

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.

def visit(node : NilableCast)

@ysbaddaden
Copy link
Contributor Author

ysbaddaden commented Apr 15, 2024

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
Copy link
Member

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

@ysbaddaden
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@ysbaddaden ysbaddaden linked a pull request Apr 16, 2024 that will close this issue
3 tasks
@straight-shoota
Copy link
Member

Related: #11058

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants