Templatize getattrInternal

src/capi/object.cpp

@@ -534,7 +534,7 @@ extern "C" int PyObject_SetAttrString(PyObject* v, const char* name, PyObject* w
 
 extern "C" PyObject* PyObject_GetAttrString(PyObject* o, const char* attr) noexcept {
     try {
-        Box* r = getattrInternal(o, internStringMortal(attr), NULL);
+        Box* r = getattrInternal<ExceptionStyle::CXX>(o, internStringMortal(attr), NULL);
         if (!r)
             PyErr_Format(PyExc_AttributeError, "'%.50s' object has no attribute '%.400s'", o->cls->tp_name, attr);
         return r;

src/core/types.h

@@ -68,6 +68,13 @@ enum class EffortLevel {
     MAXIMAL = 3,
 };
 
+namespace ExceptionStyle {
+enum ExceptionStyle {
+    CAPI,
+    CXX,
+};
+};
+
 class CompilerType;
 template <class V> class ValuedCompilerType;
 typedef ValuedCompilerType<llvm::Value*> ConcreteCompilerType;

src/runtime/builtin_modules/builtins.cpp

@@ -452,24 +452,14 @@ Box* getattrFunc(Box* obj, Box* _str, Box* default_value) {
         raiseExcHelper(TypeError, "getattr(): attribute name must be string");
     }
 
-    BoxedString* str = static_cast<BoxedString*>(_str);
-    internStringMortalInplace(str);
-
-    Box* rtn = NULL;
-    try {
-        rtn = getattrInternal(obj, str, NULL);
-    } catch (ExcInfo e) {
-        if (!e.matches(AttributeError))
-            throw e;
-    }
-
-    if (!rtn) {
-        if (default_value)
+    Box* rtn = PyObject_GetAttr(obj, _str);
+    if (rtn == NULL && default_value != NULL && PyErr_ExceptionMatches(AttributeError)) {
+        PyErr_Clear();
         return default_value;
-        else
-            raiseExcHelper(AttributeError, "'%s' object has no attribute '%s'", getTypeName(obj), str->data());
     }
 
+    if (!rtn)
+        throwCAPIException();
     return rtn;
 }
 
@@ -499,7 +489,7 @@ Box* hasattr(Box* obj, Box* _str) {
 
     Box* attr;
     try {
-        attr = getattrInternal(obj, str, NULL);
+        attr = getattrInternal<ExceptionStyle::CXX>(obj, str, NULL);
     } catch (ExcInfo e) {
         if (e.matches(Exception))
             return False;

src/runtime/capi.cpp

@@ -204,13 +204,29 @@ done:
 }
 
 
-extern "C" PyObject* PyObject_GetAttr(PyObject* o, PyObject* attr_name) noexcept {
-    try {
-        return getattrMaybeNonstring(o, attr_name);
-    } catch (ExcInfo e) {
-        setCAPIException(e);
+extern "C" PyObject* PyObject_GetAttr(PyObject* o, PyObject* attr) noexcept {
+    if (!PyString_Check(attr)) {
+        if (PyUnicode_Check(attr)) {
+            attr = _PyUnicode_AsDefaultEncodedString(attr, NULL);
+            if (attr == NULL)
                 return NULL;
+        } else {
+            PyErr_Format(TypeError, "attribute name must be string, not '%.200s'", Py_TYPE(attr)->tp_name);
+            return NULL;
+        }
+    }
+
+    BoxedString* s = static_cast<BoxedString*>(attr);
+    internStringMortalInplace(s);
+
+    Box* r = getattrInternal<ExceptionStyle::CAPI>(o, s, NULL);
+
+    if (!r && !PyErr_Occurred()) {
+        PyErr_Format(PyExc_AttributeError, "'%.50s' object has no attribute '%.400s'", o->cls->tp_name,
+                     PyString_AS_STRING(attr));
     }
+
+    return r;
 }
 
 extern "C" PyObject* PyObject_GenericGetAttr(PyObject* o, PyObject* name) noexcept {

src/runtime/descr.cpp

@@ -46,7 +46,7 @@ static void propertyDocCopy(BoxedProperty* prop, Box* fget) {
 
     static BoxedString* doc_str = internStringImmortal("__doc__");
     try {
-        get_doc = getattrInternal(fget, doc_str, NULL);
+        get_doc = getattrInternal<ExceptionStyle::CXX>(fget, doc_str, NULL);
     } catch (ExcInfo e) {
         if (!e.matches(Exception)) {
             throw e;

src/runtime/dict.cpp

@@ -588,7 +588,7 @@ Box* dictUpdate(BoxedDict* self, BoxedTuple* args, BoxedDict* kwargs) {
     if (args->size()) {
         Box* arg = args->elts[0];
         static BoxedString* keys_str = internStringImmortal("keys");
-        if (getattrInternal(arg, keys_str, NULL)) {
+        if (getattrInternal<ExceptionStyle::CXX>(arg, keys_str, NULL)) {
             dictMerge(self, arg);
         } else {
             dictMergeFromSeq2(self, arg);

src/runtime/import.cpp

@@ -382,7 +382,7 @@ static Box* importSub(const std::string& name, const std::string& full_name, Box
         path_list = NULL;
     } else {
         static BoxedString* path_str = internStringImmortal("__path__");
-        path_list = static_cast<BoxedList*>(getattrInternal(parent_module, path_str, NULL));
+        path_list = static_cast<BoxedList*>(getattrInternal<ExceptionStyle::CXX>(parent_module, path_str, NULL));
         if (path_list == NULL || path_list->cls != list_cls) {
             return None;
         }
@@ -562,7 +562,7 @@ static void ensureFromlist(Box* module, Box* fromlist, std::string& buf, bool re
     static BoxedString* path_str = internStringImmortal("__path__");
     Box* pathlist = NULL;
     try {
-        pathlist = getattrInternal(module, path_str, NULL);
+        pathlist = getattrInternal<ExceptionStyle::CXX>(module, path_str, NULL);
     } catch (ExcInfo e) {
         if (!e.matches(AttributeError))
             throw e;
@@ -584,14 +584,14 @@ static void ensureFromlist(Box* module, Box* fromlist, std::string& buf, bool re
                 continue;
 
             static BoxedString* all_str = internStringImmortal("__all__");
-            Box* all = getattrInternal(module, all_str, NULL);
+            Box* all = getattrInternal<ExceptionStyle::CXX>(module, all_str, NULL);
             if (all) {
                 ensureFromlist(module, all, buf, true);
             }
             continue;
         }
 
-        Box* attr = getattrInternal(module, s, NULL);
+        Box* attr = getattrInternal<ExceptionStyle::CXX>(module, s, NULL);
         if (attr != NULL)
             continue;
 

src/runtime/objmodel.cpp

@@ -59,6 +59,9 @@
 
 namespace pyston {
 
+using namespace pyston::ExceptionStyle;
+using pyston::ExceptionStyle::ExceptionStyle;
+
 static const std::string delitem_str("__delitem__");
 static const std::string getattribute_str("__getattribute__");
 static const std::string getattr_str("__getattr__");
@@ -196,7 +199,7 @@ extern "C" bool softspace(Box* b, bool newval) {
     bool r;
     Box* gotten = NULL;
     try {
-        Box* gotten = getattrInternal(b, softspace_str, NULL);
+        Box* gotten = getattrInternal<CXX>(b, softspace_str, NULL);
         if (!gotten) {
             r = 0;
         } else {
@@ -1470,10 +1473,16 @@ Box* dataDescriptorInstanceSpecialCases(GetattrRewriteArgs* rewrite_args, BoxedS
     return NULL;
 }
 
+template <enum ExceptionStyle S>
 Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args, bool cls_only, bool for_call,
                        Box** bind_obj_out, RewriterVar** r_bind_obj_out) {
     assert(gc::isValidGCObject(attr));
 
+    if (S == CAPI) {
+        assert(!rewrite_args && "implement me");
+        rewrite_args = NULL;
+    }
+
     if (!cls_only) {
         BoxedClass* cls = obj->cls;
 
@@ -1486,10 +1495,20 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
             STAT_TIMER(t0, "us_timer_slowpath_tpgetattro", 10);
 
             if (obj->cls->tp_getattro == slot_tp_getattr_hook) {
+                if (S == CAPI) {
+                    assert(!rewrite_args && "implement me");
+                    return obj->cls->tp_getattro(obj, attr);
+                } else {
                     return slotTpGetattrHookInternal(obj, attr, rewrite_args);
                 }
+            }
 
             Box* r = obj->cls->tp_getattro(obj, attr);
+            if (S == CAPI) {
+                assert(!rewrite_args && "implement me");
+                return r;
+            }
+
             if (!r)
                 throwCAPIException();
 
@@ -1517,18 +1536,33 @@ Box* getattrInternalEx(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_
             STAT_TIMER(t0, "us_timer_slowpath_tpgetattr", 10);
 
             assert(attr->data()[attr->size()] == '\0');
+
             Box* r = obj->cls->tp_getattr(obj, const_cast<char*>(attr->data()));
+
+            if (S == CAPI) {
+                return r;
+            } else {
                 if (!r)
                     throwCAPIException();
                 return r;
             }
         }
+    }
 
+    if (S == CAPI) {
+        try {
             return getattrInternalGeneric(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
+        } catch (ExcInfo e) {
+            setCAPIException(e);
+            return NULL;
+        }
+    } else {
+        return getattrInternalGeneric(obj, attr, rewrite_args, cls_only, for_call, bind_obj_out, r_bind_obj_out);
+    }
 }
 
 inline Box* getclsattrInternal(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args) {
-    return getattrInternalEx(obj, attr, rewrite_args,
+    return getattrInternalEx<CXX>(obj, attr, rewrite_args,
                                   /* cls_only */ true,
                                   /* for_call */ false, NULL, NULL);
 }
@@ -1935,7 +1969,7 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
     // TODO this shouldn't go here; it should be in instancemethod_cls->tp_getattr[o]
     if (obj->cls == instancemethod_cls) {
         assert(!rewrite_args || !rewrite_args->out_success);
-        return getattrInternalEx(static_cast<BoxedInstanceMethod*>(obj)->func, attr, NULL, cls_only, for_call,
+        return getattrInternalEx<CXX>(static_cast<BoxedInstanceMethod*>(obj)->func, attr, NULL, cls_only, for_call,
                                       bind_obj_out, NULL);
     }
 
@@ -1945,12 +1979,16 @@ Box* getattrInternalGeneric(Box* obj, BoxedString* attr, GetattrRewriteArgs* rew
     return NULL;
 }
 
-Box* getattrInternal(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args) {
-    return getattrInternalEx(obj, attr, rewrite_args,
+template <enum ExceptionStyle S> Box* getattrInternal(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args) {
+    return getattrInternalEx<S>(obj, attr, rewrite_args,
                                 /* cls_only */ false,
                                 /* for_call */ false, NULL, NULL);
 }
 
+// Force instantiation of the template
+template Box* getattrInternal<CAPI>(Box*, BoxedString*, GetattrRewriteArgs*);
+template Box* getattrInternal<CXX>(Box*, BoxedString*, GetattrRewriteArgs*);
+
 Box* getattrMaybeNonstring(Box* obj, Box* attr) {
     if (!PyString_Check(attr)) {
         if (PyUnicode_Check(attr)) {
@@ -1964,7 +2002,11 @@ Box* getattrMaybeNonstring(Box* obj, Box* attr) {
 
     BoxedString* s = static_cast<BoxedString*>(attr);
     internStringMortalInplace(s);
-    return getattr(obj, s);
+
+    Box* r = getattrInternal<CXX>(obj, s, NULL);
+    if (!r)
+        raiseAttributeError(obj, s->s());
+    return r;
 }
 
 extern "C" Box* getattr(Box* obj, BoxedString* attr) {
@@ -2018,7 +2060,7 @@ extern "C" Box* getattr(Box* obj, BoxedString* attr) {
         else
             dest = rewriter->getReturnDestination();
         GetattrRewriteArgs rewrite_args(rewriter.get(), rewriter->getArg(0), dest);
-        val = getattrInternal(obj, attr, &rewrite_args);
+        val = getattrInternal<CXX>(obj, attr, &rewrite_args);
 
         if (rewrite_args.out_success) {
             if (!val) {
@@ -2040,7 +2082,7 @@ extern "C" Box* getattr(Box* obj, BoxedString* attr) {
             }
         }
     } else {
-        val = getattrInternal(obj, attr, NULL);
+        val = getattrInternal<CXX>(obj, attr, NULL);
     }
 
     if (val) {
@@ -2885,14 +2927,14 @@ extern "C" Box* callattrInternal(Box* obj, BoxedString* attr, LookupScope scope,
     RewriterVar* r_val = NULL;
     if (rewrite_args) {
         GetattrRewriteArgs grewrite_args(rewrite_args->rewriter, rewrite_args->obj, Location::any());
-        val = getattrInternalEx(obj, attr, &grewrite_args, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
+        val = getattrInternalEx<CXX>(obj, attr, &grewrite_args, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
         if (!grewrite_args.out_success) {
             rewrite_args = NULL;
         } else if (val) {
             r_val = grewrite_args.out_rtn;
         }
     } else {
-        val = getattrInternalEx(obj, attr, NULL, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
+        val = getattrInternalEx<CXX>(obj, attr, NULL, scope == CLASS_ONLY, true, &bind_obj, &r_bind_obj);
     }
 
     if (val == NULL) {
@@ -5370,7 +5412,7 @@ void setGlobal(Box* globals, BoxedString* name, Box* value) {
 extern "C" Box* importFrom(Box* _m, BoxedString* name) {
     STAT_TIMER(t0, "us_timer_importFrom", 10);
 
-    Box* r = getattrInternal(_m, name, NULL);
+    Box* r = getattrInternal<CXX>(_m, name, NULL);
     if (r)
         return r;
 

src/runtime/objmodel.h

@@ -134,6 +134,7 @@ Box* compareInternal(Box* lhs, Box* rhs, int op_type, CompareRewriteArgs* rewrit
 
 // This is the equivalent of PyObject_GetAttr. Unlike getattrInternalGeneric, it checks for custom __getattr__ or
 // __getattribute__ methods.
+template <ExceptionStyle::ExceptionStyle S>
 Box* getattrInternal(Box* obj, BoxedString* attr, GetattrRewriteArgs* rewrite_args);
 
 // This is the equivalent of PyObject_GenericGetAttr, which performs the default lookup rules for getattr() (check for

src/runtime/types.cpp

@@ -1654,7 +1654,7 @@ static Box* instancemethodRepr(Box* b) {
     const char* sfuncname = "?", * sklassname = "?";
 
     static BoxedString* name_str = internStringImmortal("__name__");
-    funcname = getattrInternal(func, name_str, NULL);
+    funcname = getattrInternal<ExceptionStyle::CXX>(func, name_str, NULL);
 
     if (funcname != NULL) {
         if (!PyString_Check(funcname)) {
@@ -1666,7 +1666,7 @@ static Box* instancemethodRepr(Box* b) {
     if (klass == NULL) {
         klassname = NULL;
     } else {
-        klassname = getattrInternal(klass, name_str, NULL);
+        klassname = getattrInternal<ExceptionStyle::CXX>(klass, name_str, NULL);
         if (klassname != NULL) {
             if (!PyString_Check(klassname)) {
                 klassname = NULL;