Skip to content

Commit

Permalink
Merge pull request skulpt#859 from rixner/attr-pr
Browse files Browse the repository at this point in the history 
Normalize get/set attribute infrastructure to take Python attribute names
  • Loading branch information
@bnmnetp
bnmnetp authored May 18, 2019
2 parents 1dabfcb + 31ce72f commit 56606ae
Show file tree
Hide file tree
Showing 19 changed files with 247 additions and 193 deletions.
32 changes: 17 additions & 15 deletions src/abstract.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ Sk.abstr.sequenceContains = function (seq, ob, canSuspend) {
* Look for special method and call it, we have to distinguish between built-ins and
* python objects
*/
special = Sk.abstr.lookupSpecial(seq, "__contains__");
special = Sk.abstr.lookupSpecial(seq, Sk.builtin.str.$contains);
if (special != null) {
// method on builtin, provide this arg
return Sk.misceval.isTrue(Sk.misceval.callsimArray(special, [seq, ob]));
Expand Down Expand Up @@ -676,7 +676,7 @@ Sk.abstr.objectFormat = function (obj, format_spec) {
var result; // PyObject

// Find the (unbound!) __format__ method (a borrowed reference)
meth = Sk.abstr.lookupSpecial(obj, "__format__");
meth = Sk.abstr.lookupSpecial(obj, Sk.builtin.str.$format);
if (meth == null) {
throw new Sk.builtin.TypeError("Type " + Sk.abstr.typeName(obj) + " doesn't define __format__");
}
Expand Down Expand Up @@ -796,21 +796,22 @@ Sk.abstr.objectSetItem = function (o, key, v, canSuspend) {
goog.exportSymbol("Sk.abstr.objectSetItem", Sk.abstr.objectSetItem);


Sk.abstr.gattr = function (obj, nameJS, canSuspend) {
Sk.abstr.gattr = function (obj, pyName, canSuspend) {
var ret, f;
var objname = Sk.abstr.typeName(obj);
var jsName = pyName.$jsstr();

if (obj === null) {
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + nameJS + "'");
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + jsName + "'");
}

if (obj.tp$getattr !== undefined) {
ret = obj.tp$getattr(nameJS, canSuspend);
ret = obj.tp$getattr(pyName, canSuspend);
}

ret = Sk.misceval.chain(ret, function(r) {
if (r === undefined) {
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + nameJS + "'");
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + jsName + "'");
}
return r;
});
Expand All @@ -820,17 +821,18 @@ Sk.abstr.gattr = function (obj, nameJS, canSuspend) {
goog.exportSymbol("Sk.abstr.gattr", Sk.abstr.gattr);


Sk.abstr.sattr = function (obj, nameJS, data, canSuspend) {
Sk.abstr.sattr = function (obj, pyName, data, canSuspend) {
var objname = Sk.abstr.typeName(obj), r, setf;
var jsName = pyName.$jsstr();

if (obj === null) {
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + nameJS + "'");
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + jsName + "'");
}

if (obj.tp$setattr !== undefined) {
return obj.tp$setattr(nameJS, data, canSuspend);
return obj.tp$setattr(pyName, data, canSuspend);
} else {
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + nameJS + "'");
throw new Sk.builtin.AttributeError("'" + objname + "' object has no attribute '" + jsName + "'");
}
};
goog.exportSymbol("Sk.abstr.sattr", Sk.abstr.sattr);
Expand Down Expand Up @@ -870,7 +872,7 @@ Sk.abstr.iter = function(obj) {
var seqIter = function (obj) {
this.idx = 0;
this.myobj = obj;
this.getitem = Sk.abstr.lookupSpecial(obj, "__getitem__");
this.getitem = Sk.abstr.lookupSpecial(obj, Sk.builtin.str.$getitem);
this.tp$iternext = function () {
var ret;
try {
Expand All @@ -888,7 +890,7 @@ Sk.abstr.iter = function(obj) {
};

if (obj.tp$getattr) {
iter = Sk.abstr.lookupSpecial(obj,"__iter__");
iter = Sk.abstr.lookupSpecial(obj, Sk.builtin.str.$iter);
if (iter) {
ret = Sk.misceval.callsimArray(iter, [obj]);
if (ret.tp$iternext) {
Expand All @@ -904,7 +906,7 @@ Sk.abstr.iter = function(obj) {
}
} catch (e) { }
}
getit = Sk.abstr.lookupSpecial(obj, "__getitem__");
getit = Sk.abstr.lookupSpecial(obj, Sk.builtin.str.$getitem);
if (getit) {
// create internal iterobject if __getitem__
return new seqIter(obj);
Expand All @@ -920,7 +922,7 @@ goog.exportSymbol("Sk.abstr.iter", Sk.abstr.iter);
*
* @returns {null|Object} Return null if not found or the function
*/
Sk.abstr.lookupSpecial = function(op, str) {
Sk.abstr.lookupSpecial = function(op, pyName) {
var res;
var obtp;
if (op.ob$type) {
Expand All @@ -929,7 +931,7 @@ Sk.abstr.lookupSpecial = function(op, str) {
return null;
}

return Sk.builtin.type.typeLookup(obtp, str);
return Sk.builtin.type.typeLookup(obtp, pyName);
};
goog.exportSymbol("Sk.abstr.lookupSpecial", Sk.abstr.lookupSpecial);

Expand Down
35 changes: 19 additions & 16 deletions src/builtin.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ Sk.builtin.round = function round (number, ndigits) {
}

// try calling internal magic method
special = Sk.abstr.lookupSpecial(number, "__round__");
special = Sk.abstr.lookupSpecial(number, Sk.builtin.str.$round);
if (special != null) {
// method on builtin, provide this arg
if (!Sk.builtin.checkFunction(number)) {
Expand Down Expand Up @@ -274,7 +274,7 @@ Sk.builtin.len = function len (item) {

if (item.tp$length) {
if (Sk.builtin.checkFunction(item)) {
special = Sk.abstr.lookupSpecial(item, "__len__");
special = Sk.abstr.lookupSpecial(item, Sk.builtin.str.$len);
if (special != null) {
return Sk.misceval.callsimArray(special, [item]);
} else {
Expand Down Expand Up @@ -475,7 +475,7 @@ Sk.builtin.abs = function abs (x) {

// call custom __abs__ methods
if (x.tp$getattr) {
var f = x.tp$getattr("__abs__");
var f = x.tp$getattr(Sk.builtin.str.$abs);
return Sk.misceval.callsimArray(f);
}

Expand Down Expand Up @@ -622,7 +622,7 @@ Sk.builtin.dir = function dir (x) {
var _seq;

// try calling magic method
var special = Sk.abstr.lookupSpecial(x, "__dir__");
var special = Sk.abstr.lookupSpecial(x, Sk.builtin.str.$dir);
if(special != null) {
// method on builtin, provide this arg
_seq = Sk.misceval.callsimArray(special, [x]);
Expand Down Expand Up @@ -819,36 +819,39 @@ Sk.builtin.hash = function hash (value) {
// todo; throw properly for unhashable types
};

Sk.builtin.getattr = function getattr (obj, name, default_) {
var ret, mangledName;
Sk.builtin.getattr = function getattr (obj, pyName, default_) {
var ret, mangledName, jsName;
Sk.builtin.pyCheckArgsLen("getattr", arguments.length, 2, 3);
if (!Sk.builtin.checkString(name)) {
if (!Sk.builtin.checkString(pyName)) {
throw new Sk.builtin.TypeError("attribute name must be string");
}

mangledName = Sk.fixReservedWords(Sk.ffi.remapToJs(name));
jsName = pyName.$jsstr();
mangledName = new Sk.builtin.str(Sk.fixReservedWords(jsName));
ret = obj.tp$getattr(mangledName);
if (ret === undefined) {
if (default_ !== undefined) {
return default_;
} else {
throw new Sk.builtin.AttributeError("'" + Sk.abstr.typeName(obj) + "' object has no attribute '" + name.v + "'");
throw new Sk.builtin.AttributeError("'" + Sk.abstr.typeName(obj) + "' object has no attribute '" + jsName + "'");
}
}
return ret;
};

Sk.builtin.setattr = function setattr (obj, name, value) {
Sk.builtin.setattr = function setattr (obj, pyName, value) {
var jsName;
Sk.builtin.pyCheckArgsLen("setattr", arguments.length, 3, 3);
// cannot set or del attr from builtin type
if (obj === undefined || obj["$r"] === undefined || obj["$r"]().v.slice(1,5) !== "type") {
if (!Sk.builtin.checkString(name)) {
if (!Sk.builtin.checkString(pyName)) {
throw new Sk.builtin.TypeError("attribute name must be string");
}
jsName = pyName.$jsstr();
if (obj.tp$setattr) {
obj.tp$setattr(Sk.fixReservedWords(Sk.ffi.remapToJs(name)), value);
obj.tp$setattr(new Sk.builtin.str(Sk.fixReservedWords(jsName)), value);
} else {
throw new Sk.builtin.AttributeError("object has no attribute " + Sk.ffi.remapToJs(name));
throw new Sk.builtin.AttributeError("object has no attribute " + jsName);
}
return Sk.builtin.none.none$;
}
Expand Down Expand Up @@ -1064,7 +1067,7 @@ Sk.builtin.hasattr = function hasattr (obj, attr) {
}

if (obj.tp$getattr) {
if (obj.tp$getattr(attr.v)) {
if (obj.tp$getattr(attr)) {
return Sk.builtin.bool.true$;
} else {
return Sk.builtin.bool.false$;
Expand Down Expand Up @@ -1246,7 +1249,7 @@ Sk.builtin.format = function format (value, format_spec) {
Sk.builtin.reversed = function reversed (seq) {
Sk.builtin.pyCheckArgsLen("reversed", arguments.length, 1, 1);

var special = Sk.abstr.lookupSpecial(seq, "__reversed__");
var special = Sk.abstr.lookupSpecial(seq, Sk.builtin.str.$reversed);
if (special != null) {
return Sk.misceval.callsimArray(special, [seq]);
} else {
Expand All @@ -1262,7 +1265,7 @@ Sk.builtin.reversed = function reversed (seq) {
var reverseIter = function (obj) {
this.idx = obj.sq$length() - 1;
this.myobj = obj;
this.getitem = Sk.abstr.lookupSpecial(obj, "__getitem__");
this.getitem = Sk.abstr.lookupSpecial(obj, Sk.builtin.str.$getitem);
this.tp$iter = function() {
return this;
},
Expand Down
31 changes: 16 additions & 15 deletions src/compile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,12 +395,12 @@ Compiler.prototype.ctuplelistorset = function(e, data, tuporlist) {
items = [];
for (i = 0; i < e.elts.length; ++i) {
item = this.vexpr(e.elts[i]);
// The following is an ugly check to see if item was
// turned into a constant. As vexpr returns a string,
// this requires seeing if "$const" is contained
// within it. A better solution would require a
// change to vexpr, which would be more invasive.

// The following is an ugly check to see if item was
// turned into a constant. As vexpr returns a string,
// this requires seeing if "$const" is contained
// within it. A better solution would require a
// change to vexpr, which would be more invasive.
if (allconsts && (item.indexOf('$const') == -1)) {
allconsts = false;
}
Expand Down Expand Up @@ -725,7 +725,7 @@ Compiler.prototype.cboolop = function (e) {
* (already vexpr'ed, so we can evaluate it once and reuse for both load and store ops)
*/
Compiler.prototype.vexpr = function (e, data, augvar, augsubs) {
var mangled;
var mangled, mname;
var val;
var result;
var nStr; // used for preserving signs for floats (zeros)
Expand Down Expand Up @@ -797,13 +797,14 @@ Compiler.prototype.vexpr = function (e, data, augvar, augsubs) {
mangled = mangleName(this.u.private_, new Sk.builtin.str(mangled)).v;
mangled = fixReservedWords(mangled);
mangled = fixReservedNames(mangled);
mname = this.makeConstant("new Sk.builtin.str('" + mangled + "')");
switch (e.ctx) {
case AugLoad:
out("$ret = Sk.abstr.gattr(", augvar, ",'", mangled, "', true);");
out("$ret = Sk.abstr.gattr(", augvar, ",", mname, ", true);");
this._checkSuspension(e);
return this._gr("lattr", "$ret");
case Load:
out("$ret = Sk.abstr.gattr(", val, ",'", mangled, "', true);");
out("$ret = Sk.abstr.gattr(", val, ",", mname, ", true);");
this._checkSuspension(e);
return this._gr("lattr", "$ret");
case AugStore:
Expand All @@ -812,12 +813,12 @@ Compiler.prototype.vexpr = function (e, data, augvar, augsubs) {
// so this will never *not* execute. But it could, if Sk.abstr.numberInplaceBinOp were fixed.
out("$ret = undefined;");
out("if(", data, "!==undefined){");
out("$ret = Sk.abstr.sattr(", augvar, ",'", mangled, "',", data, ", true);");
out("$ret = Sk.abstr.sattr(", augvar, ",", mname, ",", data, ", true);");
out("}");
this._checkSuspension(e);
break;
case Store:
out("$ret = Sk.abstr.sattr(", val, ",'", mangled, "',", data, ", true);");
out("$ret = Sk.abstr.sattr(", val, ",", mname, ",", data, ", true);");
this._checkSuspension(e);
break;
case Del:
Expand Down Expand Up @@ -1473,13 +1474,13 @@ Compiler.prototype.cwith = function (s) {
mgr = this._gr("mgr", this.vexpr(s.context_expr));

// exit = mgr.__exit__
out("$ret = Sk.abstr.gattr(",mgr,",'__exit__', true);");
out("$ret = Sk.abstr.gattr(",mgr,",Sk.builtin.str.$exit, true);");
this._checkSuspension(s);
exit = this._gr("exit", "$ret");
this.u.tempsToSave.push(exit);

// value = mgr.__enter__()
out("$ret = Sk.abstr.gattr(",mgr,",'__enter__', true);");
out("$ret = Sk.abstr.gattr(",mgr,",Sk.builtin.str.$enter, true);");
this._checkSuspension(s);
out("$ret = Sk.misceval.callsimOrSuspendArray($ret);");
this._checkSuspension(s);
Expand Down Expand Up @@ -1559,7 +1560,7 @@ Compiler.prototype.cimportas = function (name, asname, mod) {
while (dotLoc !== -1) {
dotLoc = src.indexOf(".");
attr = dotLoc !== -1 ? src.substr(0, dotLoc) : src;
cur = this._gr("lattr", "Sk.abstr.gattr(", cur, ",'", attr, "')");
cur = this._gr("lattr", "Sk.abstr.gattr(", cur, ", new Sk.builtin.str('", attr, "'))");
src = src.substr(dotLoc + 1);
}
}
Expand Down Expand Up @@ -1628,7 +1629,7 @@ Compiler.prototype.cfromimport = function (s) {
}

//out("print(\"getting Sk.abstr.gattr(", mod, ",", alias.name["$r"]().v, ")\");");
got = this._gr("item", "Sk.abstr.gattr(", mod, ",", aliasOut, ")");
got = this._gr("item", "Sk.abstr.gattr(", mod, ", new Sk.builtin.str(", aliasOut, "))");
//out("print('got');");
storeName = alias.name;
if (alias.asname) {
Expand Down
Loading

0 comments on commit 56606ae

Please sign in to comment.