search

oracle / graalpython

A Python 3 implementation built on GraalVM
Other
1.2k stars 104 forks source link

Adding hasTag method to write local variable node class, and python call nodes taking one to four arguments. #281

Closed Yuhala closed 6 months ago

Yuhala commented 1 year ago

The com.oracle.graal.python.nodes.frame.WriteLocalVariableNode class does not override the hasTag method which provides the corresponding instrumentation tags. Same issue with the four nested classes: PythonCallUnary, PythonCallBinary, PythonCallTernary and PythonCallQuaternary of the com.oracle.graal.python.nodes.call.PythonCallNode class.

The corresponding issue is: https://github.com/oracle/graalpython/issues/280

The proposed fixes in this PR override the hasTag method in those classes.

oracle-contributor-agreement[bot] commented 1 year ago

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA). The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When singing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

msimacek commented 1 year ago

Hi @Yuhala, thank you for your interest in GraalPy. Unfortunately, for the next release we did a major rework of our interpreter internals (from AST interpreter to bytecode interpreter), so instrumentation is done differently and your fix won't work. Could you please share how did you run into the issue and what do you use the write variable tags for? We can come up with a different fix.

Yuhala commented 1 year ago

Hello @msimacek, thanks for your response. I am building a Truffle instrumentation tool for GraalPython, to be used for a research project. I tried instrumenting python local variable writes and some function/method calls (with one to four arguments). I realised my wrapper nodes weren't working as expected, and after a quick check in the GraalPython source code, I noticed the corresponding instrumentation tags weren't exposed by those nodes. So my quick fix was to override the hasTag method for those classes.

timfel commented 6 months ago

Closing as stale.