-
Notifications
You must be signed in to change notification settings - Fork 323
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
Avoid Interop TypeOfNode specializations for EnsoObject #9431
Avoid Interop TypeOfNode specializations for EnsoObject #9431
Conversation
.../runtime/src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for such a quick fix!
} | ||
|
||
@ExportMessage | ||
Type getType(@Bind("$node") Node node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per suggestion of Christian Humber:
you should use @Bind("$node") for context references in libraries instead of arbitrary nodes. In general they might not be adoptable:
@ExportLibrary(value = TypesLibrary.class, receiverType = Long.class)
public static class DefaultLongExports {
@ExportMessage
static boolean hasType(Long receiver) {
return true;
}
@ExportMessage
static Class<?> getType(Long receiver, @Bind("$node") Node node) {
SLContext.get(node);
return Long.class;
}
}
we don't have to use @Cached("1") int ignore anymore...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... if it works. Sometimes it doesn't work and I had to add the @Cached("1") int ignore
trick back.
.../runtime/src/test/java/org/enso/interpreter/node/expression/builtin/meta/TypeOfNodeTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we have the same problem with long
and double
?
I've added |
Pull Request Description
Fixing inconsistency in
@Specialization
inTypeOfNode
discovered by Radek. Testing the node in two situations - directly and after being primed to access interopTruffleObject
first.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Java,
style guides.- All code has been tested: