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

Object methods use incorrect this #172

Closed
wiwichips opened this issue Sep 20, 2023 · 3 comments · Fixed by #224 or #227
Closed

Object methods use incorrect this #172

wiwichips opened this issue Sep 20, 2023 · 3 comments · Fixed by #224 or #227
Assignees
Labels
bug Something isn't working

Comments

@wiwichips
Copy link
Collaborator

Issue type

Bug

How did you install PythonMonkey?

Installed from pip

OS platform and distribution

Ubuntu 22.04

Python version (python --version)

Python 3.10.12

PythonMonkey version (pip show pythonmonkey)

0.2.1

Bug Description

When calling the method of a class, this in the scope of the function is globalThis and not the function's this

example.js

// old style class
function Rectangle(w, h) {
  this.w = w;
  this.h = h
}
 
Rectangle.prototype = {
  getThis: function () {
    return this;
  },
  getArea: function () {
    return this.w * this.h;
  },
};

// es5 class
class Rectangle2 {
  constructor(w,h) {
    this.w = w;
    this.h = h;
  }

  getThis() {
    return this;
  }

  getArea() {
    return this.w * this.h;
  }
}

module.exports.Rectangle = Rectangle;
module.exports.Rectangle2 = Rectangle2;

main.py

import pythonmonkey as pm

example = pm.require('./example')
r = pm.new(example.Rectangle)(1,2)

print(r.getArea()) # nan, should be 2
print(r.getThis()) # globalThis, should be { w: 1, h: 2 }

###
r2 = pm.new(example.Rectangle2)(1,2)

print(r2.getArea()) # nan, should be 2
print(r2.getThis()) # globalThis, should be { w: 1, h: 2 }

--- When I run python3 main.py I get the following output:

nan
{'python': {'pythonMonkey': {'dir': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey', 'isCompilableUnit': <built-in function isCompilableUnit>, 'nodeModules': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey/node_modules'}, 'stdout': {'write': <function <lambda> at 0x7fec94386710>, 'read': <function <lambda> at 0x7fec90642290>}, 'stderr': {'write': <function <lambda> at 0x7fec9079a830>, 'read': <function <lambda> at 0x7fec90642320>}, 'print': <built-in function print>, 'eval': <built-in function eval>, 'exec': <built-in function exec>, 'getenv': <function getenv at 0x7fec94373880>, 'paths': ['/home/will/test/pm-bug/getArea', '/usr/lib/python310.zip', '/usr/lib/python3.10', '/usr/lib/python3.10/lib-dynload', '/home/will/.local/lib/python3.10/site-packages', '/usr/local/lib/python3.10/dist-packages', '/usr/lib/python3/dist-packages'], 'exit': <built-in method JSFunctionCallable of tuple object at 0x7fec90458340>, 'load': <function load at 0x7fec90642560>}, 'pmEval': <built-in function eval>, 'bootstrap': {'modules': {'vm': {'runInContext': <built-in method JSFunctionCallable of tuple object at 0x7fec904584c0>}, 'ctx-module': {'makeNodeProgramContext': <built-in method JSFunctionCallable of tuple object at 0x7fec90458540>, 'CtxModule': <built-in method JSFunctionCallable of tuple object at 0x7fec90458500>}, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec90458480>, 'fs': {'constants': {'S_IFDIR': 16384.0}, 'statSync': <built-in method JSFunctionCallable of tuple object at 0x7fec90458580>, 'statSync_inner': <function statSync_inner at 0x7fec906423b0>, 'readFileSync': <function readFileSync at 0x7fec90642440>, 'existsSync': <function existsSync at 0x7fec906424d0>}}, 'require': <built-in method JSFunctionCallable of tuple object at 0x7fec90458440>, 'builtinModules': {'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec90458380>}}, 'console': {'log': <built-in method JSFunctionCallable of tuple object at 0x7fec90458600>, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec90458640>, 'info': <built-in method JSFunctionCallable of tuple object at 0x7fec90458680>, 'warn': <built-in method JSFunctionCallable of tuple object at 0x7fec904586c0>, 'error': <built-in method JSFunctionCallable of tuple object at 0x7fec90458700>}, 'atob': <function <lambda> at 0x7fec90439d80>, 'btoa': <function <lambda> at 0x7fec90439ea0>}
nan
{'python': {'pythonMonkey': {'dir': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey', 'isCompilableUnit': <built-in function isCompilableUnit>, 'nodeModules': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey/node_modules'}, 'stdout': {'write': <function <lambda> at 0x7fec94386710>, 'read': <function <lambda> at 0x7fec90642290>}, 'stderr': {'write': <function <lambda> at 0x7fec9079a830>, 'read': <function <lambda> at 0x7fec90642320>}, 'print': <built-in function print>, 'eval': <built-in function eval>, 'exec': <built-in function exec>, 'getenv': <function getenv at 0x7fec94373880>, 'paths': ['/home/will/test/pm-bug/getArea', '/usr/lib/python310.zip', '/usr/lib/python3.10', '/usr/lib/python3.10/lib-dynload', '/home/will/.local/lib/python3.10/site-packages', '/usr/local/lib/python3.10/dist-packages', '/usr/lib/python3/dist-packages'], 'exit': <built-in method JSFunctionCallable of tuple object at 0x7fec90458bc0>, 'load': <function load at 0x7fec90642560>}, 'pmEval': <built-in function eval>, 'bootstrap': {'modules': {'vm': {'runInContext': <built-in method JSFunctionCallable of tuple object at 0x7fec90459300>}, 'ctx-module': {'makeNodeProgramContext': <built-in method JSFunctionCallable of tuple object at 0x7fec90459400>, 'CtxModule': <built-in method JSFunctionCallable of tuple object at 0x7fec90459480>}, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec90459200>, 'fs': {'constants': {'S_IFDIR': 16384.0}, 'statSync': <built-in method JSFunctionCallable of tuple object at 0x7fec90459580>, 'statSync_inner': <function statSync_inner at 0x7fec906423b0>, 'readFileSync': <function readFileSync at 0x7fec90642440>, 'existsSync': <function existsSync at 0x7fec906424d0>}}, 'require': <built-in method JSFunctionCallable of tuple object at 0x7fec90459080>, 'builtinModules': {'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec90459780>}}, 'console': {'log': <built-in method JSFunctionCallable of tuple object at 0x7fec90459840>, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fec904598c0>, 'info': <built-in method JSFunctionCallable of tuple object at 0x7fec90459940>, 'warn': <built-in method JSFunctionCallable of tuple object at 0x7fec904599c0>, 'error': <built-in method JSFunctionCallable of tuple object at 0x7fec90459a40>}, 'atob': <function <lambda> at 0x7fec90439d80>, 'btoa': <function <lambda> at 0x7fec90439ea0>}

getArea returns nan because globalThis.w === undefined and undefined * undefined = NaN, getThis returns globalThis because this is not set to the object this.

Standalone code to reproduce the issue

import pythonmonkey as pm

pm.eval("""
function Square(w) {
  this.w = w;
}
Square.prototype = {
  getThis: function () {
    return this;
  },
};
""")

Square = pm.globalThis.Square
r = pm.new(Square)(5)

print(r.getThis()) # globalThis, but should just be { w: 5 }

Relevant log output or backtrace

{'python': {'pythonMonkey': {'dir': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey', 'isCompilableUnit': <built-in function isCompilableUnit>, 'nodeModules': '/home/will/.local/lib/python3.10/site-packages/pythonmonkey/node_modules'}, 'stdout': {'write': <function <lambda> at 0x7fb456a867a0>, 'read': <function <lambda> at 0x7fb452c42200>}, 'stderr': {'write': <function <lambda> at 0x7fb452d9a7a0>, 'read': <function <lambda> at 0x7fb452c42290>}, 'print': <built-in function print>, 'eval': <built-in function eval>, 'exec': <built-in function exec>, 'getenv': <function getenv at 0x7fb456a73880>, 'paths': ['/home/will/test/pm-bug/getArea', '/usr/lib/python310.zip', '/usr/lib/python3.10', '/usr/lib/python3.10/lib-dynload', '/home/will/.local/lib/python3.10/site-packages', '/usr/local/lib/python3.10/dist-packages', '/usr/lib/python3/dist-packages'], 'exit': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58380>, 'load': <function load at 0x7fb452c424d0>}, 'pmEval': <built-in function eval>, 'bootstrap': {'modules': {'vm': {'runInContext': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58500>}, 'ctx-module': {'makeNodeProgramContext': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58580>, 'CtxModule': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58540>}, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fb452a584c0>, 'fs': {'constants': {'S_IFDIR': 16384.0}, 'statSync': <built-in method JSFunctionCallable of tuple object at 0x7fb452a585c0>, 'statSync_inner': <function statSync_inner at 0x7fb452c42320>, 'readFileSync': <function readFileSync at 0x7fb452c423b0>, 'existsSync': <function existsSync at 0x7fb452c42440>}}, 'require': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58480>, 'builtinModules': {'debug': <built-in method JSFunctionCallable of tuple object at 0x7fb452a583c0>}}, 'console': {'log': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58640>, 'debug': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58680>, 'info': <built-in method JSFunctionCallable of tuple object at 0x7fb452a586c0>, 'warn': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58700>, 'error': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58740>}, 'atob': <function <lambda> at 0x7fb452a39cf0>, 'btoa': <function <lambda> at 0x7fb452a39e10>, 'Square': <built-in method JSFunctionCallable of tuple object at 0x7fb452a58340>}

Additional info if applicable

No response

What branch of PythonMonkey were you developing on? (If applicable)

No response

@wiwichips
Copy link
Collaborator Author

@zanxueyan ran into a byproduct of this bug described in this message here: #170 (comment)

It can be described by this minimal repro:

import pythonmonkey as pm

pm.eval("""
class Human {
  constructor(name)
  {
    this.name = name;
  }

  setName(name)
  {
    this.name = name;
  }

  getName()
  {
    return this.name;
  }
}

""")

Human = pm.eval('Human')

person = pm.new(Human)('Will')

# setting it using setName
person.setName('Joe')

print(person.getName()) # prints "Joe"
print(person.name) # prints "Will" but should print "Joe"!!!
print(person['name']) # prints "Will" but should print "Joe"!!!

# now try setting it directly 
person.name = "Liang"

print(person.getName()) # prints "Joe"
print(person.name) # prints "Liang"!!!
print(person['name']) # prints "Liang"!!!

since this in setName is actually globalThis it doesn't change the object's name!

@zollqir
Copy link
Collaborator

zollqir commented Jan 31, 2024

I don't think this issue should be closed just yet, as while the specific above minimal repro is no longer an issue, there are still cases where the "wrong" self or this is used on main. This may require a discussion on how we want pythonmonkey to behave, but to explain my thoughts (and I've spoken about this before in the past, such as here and here):

In JS, a function always has this, and that this is the object preceeding the . at the call site (obj.func(), this is obj), defaulting to globalThis if there is no . (func(), this is globalThis).

However in python, there are both functions and methods as distinct types. Python functions are equivalent to, and have similar behaviour as, JS Functions except that they do not have an equivalent to this, nor do they have an equivalent to globalThis (perhaps the closest equivalent to globalThis would be the dict returned by globals()). Methods on the other hand are akin to bound JS Functions, with self being the equivalent of this. Under the hood, a PyMethod is just a struct containing a pointer to the underlying function, and a pointer to the bound object, and when the method gets called, it in turn calls the function and prepends the bound object to the list of arguments like so:

import types

class MyClass:
  def pyMeth(self, arg1, arg2):
    print(arg1)
    print(arg2)
    return self

pyObj = MyClass()
result = pyObj.pyMeth("Hello", "World) # result is pyObj

pyDict1 = {"pyMeth":  pyObj.pyMeth}
pyDict2 = {"pyMeth": MyClass.pyMeth}
pyDict3 = {}
pyDict3["pyMeth"] = types.MethodType(MyClass.pyMeth, pyDict3)

print(pyObj.pyMeth) # <bound method MyClass.pyMethod of <pyObj>>
print(pyDict1["pyMeth"]) # <bound method MyClass.pyMethod of <pyObj>> (NOT bound to pyDict1)
print(pyDict2["pyMeth"]) # <function MyClass.pyMeth> (NOT bound to anything)
print(pyDict3["pyMeth"]) # <bound method MyClass.pyMethod of <pyDict3>> (bound to pyDict3)

result = pyDict1["pyMeth"]("Hello", "World") # result is pyObj, NOT pyDict1
# result = pyDict2["pyMeth"]("Hello", "World") # results in "missing 1 required positional argument: 'self'" TypeError
result = pyDict3["pyMeth"]("Hello", "World") # result is pyDict3

Importantly there is no such thing as an unbound method in python, a method is bound to an object immediately when it is created, and the only way to get a function bound to another object is to manually create a new method object. Below is an example of the consequences of this for pythonmonkey:

import pythonmonkey as pm

pm.eval("""
class Human {
  constructor(name)
  {
    this.name = name;
  }

  setName(name)
  { 
    this.name = name;
  }

  getName()
  {
    return this.name;
  }
}

""")

Human = pm.eval('Human')

person = pm.new(Human)('Will')

# setting it using setName
person.setName('Joe')

print(person.getName()) # prints "Joe"
print(person.name) # prints "Joe"
print(person['name']) # prints "Joe"

# now try setting it directly 
person.name = "Liang"

print(person.getName()) # prints "Liang"
print(person.name) # prints "Liang"
print(person['name']) # prints "Liang"

pyPerson = { "name": "John", "getName": person.getName, "setName": person.setName}

# setting it using setName
pyPerson["setName"]("Jeremy")

print(pyPerson["getName"]())  # prints "Jeremy"
print(pyPerson["name"])       # prints "John"

# now try setting it directly
pyPerson["name"] = "Claude"

print(pyPerson["getName"]())  # prints "Jeremy"
print(pyPerson["name"])       # prints "Claude"

My branch #227 introduces a new type called JSMethodProxy that allows the user to declare when a given function is intended to behave like a method or a function to get around this issue.

@philippedistributive
Copy link
Collaborator

@caleb-distributive let's make sure the default code resolves the issue listed initially above and the one listed in #58

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
Status: Done
3 participants