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

Missing @NodeInfo(shortName = "in") for js/nodes/binary/InNode #754

Closed
BarrensZeppelin opened this issue Aug 25, 2023 · 3 comments
Closed
Assignees
Labels
bug Something isn't working

Comments

@BarrensZeppelin
Copy link

Is there a reason why the binary InNode doesn't have a NodeInfo annotation?

public abstract class InNode extends JSBinaryNode {

Many of the other binary nodes have them, such as JSAddNode:

@NodeInfo(shortName = "+")
public abstract class JSAddNode extends JSBinaryNode implements Truncatable {

@iamstolis
Copy link
Member

Is there a reason why the binary InNode doesn't have a NodeInfo annotation?

No particular reason, we can add it, I guess. Could you, please, elaborate a bit on why do you need it/how have you discovered that it is not there? Thank you in advance.

@iamstolis iamstolis self-assigned this Aug 25, 2023
@iamstolis iamstolis added the bug Something isn't working label Aug 25, 2023
@BarrensZeppelin
Copy link
Author

Hi!
NodeProf.js uses the shortName (which is assigned to the operator key in JSBinaryNode.getNodeObject()) to identify which binary operation that is instrumented (profiled).
https://github.com/Haiyang-Sun/nodeprof.js/blob/master/src/ch.usi.inf.nodeprof/src/ch/usi/inf/nodeprof/handlers/BinaryEventHandler.java#L35

@iamstolis
Copy link
Member

Thank you for the additional details. I have added @NodeInfo annotation to InNode (and to two other nodes) in e8c3685.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants