[mypyc] Use struct access instead of native getters/setters (#8660)

After some discussions with Sully we agreed that essentially inlining all attribute access may be a good idea. For trait attribute access, this requires setting up offset tables.

The offset tables are placed after a trait vtable for every trait base.

I didn't add many tests, relying on existing ones, since this PR doesn't add any new functionality.

Author: ilevkivskyi

mypyc/codegen/emit.py

@@ -248,6 +248,21 @@ def tuple_c_declaration(self, rtuple: RTuple) -> List[str]:
 
         return result
 
+    def emit_undefined_attr_check(self, rtype: RType, attr_expr: str,
+                                  compare: str,
+                                  unlikely: bool = False) -> None:
+        if isinstance(rtype, RTuple):
+            check = '({})'.format(self.tuple_undefined_check_cond(
+                rtype, attr_expr, self.c_undefined_value, compare)
+            )
+        else:
+            check = '({} {} {})'.format(
+                attr_expr, compare, self.c_undefined_value(rtype)
+            )
+        if unlikely:
+            check = '(unlikely{})'.format(check)
+        self.emit_line('if {} {{'.format(check))
+
     def tuple_undefined_check_cond(
             self, rtuple: RTuple, tuple_expr_in_c: str,
             c_type_compare_val: Callable[[RType], str], compare: str) -> str:

mypyc/codegen/emitclass.py

@@ -6,14 +6,14 @@
 
 from mypyc.common import PREFIX, NATIVE_PREFIX, REG_PREFIX
 from mypyc.codegen.emit import Emitter, HeaderDeclaration
-from mypyc.codegen.emitfunc import native_function_header, native_getter_name, native_setter_name
+from mypyc.codegen.emitfunc import native_function_header
 from mypyc.codegen.emitwrapper import (
     generate_dunder_wrapper, generate_hash_wrapper, generate_richcompare_wrapper,
     generate_bool_wrapper, generate_get_wrapper,
 )
 from mypyc.ir.rtypes import RType, RTuple, object_rprimitive
 from mypyc.ir.func_ir import FuncIR, FuncDecl, FUNC_STATICMETHOD, FUNC_CLASSMETHOD
-from mypyc.ir.class_ir import ClassIR, VTableMethod, VTableEntries
+from mypyc.ir.class_ir import ClassIR, VTableEntries
 from mypyc.sametype import is_same_type
 from mypyc.namegen import NameGenerator
 
@@ -98,8 +98,6 @@ def generate_class_type_decl(cl: ClassIR, c_emitter: Emitter,
     generate_object_struct(cl, external_emitter)
     generate_full = not cl.is_trait and not cl.builtin_base
     if generate_full:
-        declare_native_getters_and_setters(cl, emitter)
-
         context.declarations[emitter.native_function_name(cl.ctor)] = HeaderDeclaration(
             '{};'.format(native_function_header(cl.ctor, emitter)),
             needs_export=True,
@@ -216,7 +214,6 @@ def emit_line() -> None:
         emit_line()
         generate_dealloc_for_class(cl, dealloc_name, clear_name, emitter)
         emit_line()
-        generate_native_getters_and_setters(cl, emitter)
 
         if cl.allow_interpreted_subclasses:
             shadow_vtable_name = generate_vtables(
@@ -293,64 +290,6 @@ def generate_object_struct(cl: ClassIR, emitter: Emitter) -> None:
     )
 
 
-def declare_native_getters_and_setters(cl: ClassIR,
-                                       emitter: Emitter) -> None:
-    decls = emitter.context.declarations
-    for attr, rtype in cl.attributes.items():
-        getter_name = native_getter_name(cl, attr, emitter.names)
-        setter_name = native_setter_name(cl, attr, emitter.names)
-        decls[getter_name] = HeaderDeclaration(
-            '{}{}({} *self);'.format(emitter.ctype_spaced(rtype),
-                                     getter_name,
-                                     cl.struct_name(emitter.names)),
-            needs_export=True,
-        )
-        decls[setter_name] = HeaderDeclaration(
-            'bool {}({} *self, {}value);'.format(native_setter_name(cl, attr, emitter.names),
-                                                 cl.struct_name(emitter.names),
-                                                 emitter.ctype_spaced(rtype)),
-            needs_export=True,
-        )
-
-
-def generate_native_getters_and_setters(cl: ClassIR,
-                                        emitter: Emitter) -> None:
-    for attr, rtype in cl.attributes.items():
-        attr_field = emitter.attr(attr)
-
-        # Native getter
-        emitter.emit_line('{}{}({} *self)'.format(emitter.ctype_spaced(rtype),
-                                                  native_getter_name(cl, attr, emitter.names),
-                                                  cl.struct_name(emitter.names)))
-        emitter.emit_line('{')
-        if rtype.is_refcounted:
-            emit_undefined_check(rtype, emitter, attr_field, '==')
-            emitter.emit_lines(
-                'PyErr_SetString(PyExc_AttributeError, "attribute {} of {} undefined");'.format(
-                    repr(attr), repr(cl.name)),
-                '} else {')
-            emitter.emit_inc_ref('self->{}'.format(attr_field), rtype)
-            emitter.emit_line('}')
-        emitter.emit_line('return self->{};'.format(attr_field))
-        emitter.emit_line('}')
-        emitter.emit_line()
-        # Native setter
-        emitter.emit_line(
-            'bool {}({} *self, {}value)'.format(native_setter_name(cl, attr, emitter.names),
-                                                cl.struct_name(emitter.names),
-                                                emitter.ctype_spaced(rtype)))
-        emitter.emit_line('{')
-        if rtype.is_refcounted:
-            emit_undefined_check(rtype, emitter, attr_field, '!=')
-            emitter.emit_dec_ref('self->{}'.format(attr_field), rtype)
-            emitter.emit_line('}')
-        # This steal the reference to src, so we don't need to increment the arg
-        emitter.emit_lines('self->{} = value;'.format(attr_field),
-                           'return 1;',
-                           '}')
-        emitter.emit_line()
-
-
 def generate_vtables(base: ClassIR,
                      vtable_setup_name: str,
                      vtable_name: str,
@@ -359,6 +298,18 @@ def generate_vtables(base: ClassIR,
     """Emit the vtables and vtable setup functions for a class.
 
     This includes both the primary vtable and any trait implementation vtables.
+    The trait vtables go before the main vtable, and have the following layout:
+        {
+            CPyType_T1,         // pointer to type object
+            C_T1_trait_vtable,  // pointer to array of method pointers
+            C_T1_offset_table,  // pointer to array of attribute offsets
+            CPyType_T2,
+            C_T2_trait_vtable,
+            C_T2_offset_table,
+            ...
+        }
+    The method implementations are calculated at the end of IR pass, attribute
+    offsets are {offsetof(native__C, _x1), offsetof(native__C, _y1), ...}.
 
     To account for both dynamic loading and dynamic class creation,
     vtables are populated dynamically at class creation time, so we
@@ -370,22 +321,33 @@ def generate_vtables(base: ClassIR,
 
     Returns the expression to use to refer to the vtable, which might be
     different than the name, if there are trait vtables.
-
     """
 
     def trait_vtable_name(trait: ClassIR) -> str:
         return '{}_{}_trait_vtable{}'.format(
             base.name_prefix(emitter.names), trait.name_prefix(emitter.names),
             '_shadow' if shadow else '')
 
+    def trait_offset_table_name(trait: ClassIR) -> str:
+        return '{}_{}_offset_table'.format(
+            base.name_prefix(emitter.names), trait.name_prefix(emitter.names)
+        )
+
     # Emit array definitions with enough space for all the entries
     emitter.emit_line('static CPyVTableItem {}[{}];'.format(
         vtable_name,
-        max(1, len(base.vtable_entries) + 2 * len(base.trait_vtables))))
+        max(1, len(base.vtable_entries) + 3 * len(base.trait_vtables))))
+
     for trait, vtable in base.trait_vtables.items():
+        # Trait methods entry (vtable index -> method implementation).
         emitter.emit_line('static CPyVTableItem {}[{}];'.format(
             trait_vtable_name(trait),
             max(1, len(vtable))))
+        # Trait attributes entry (attribute number in trait -> offset in actual struct).
+        emitter.emit_line('static size_t {}[{}];'.format(
+            trait_offset_table_name(trait),
+            max(1, len(trait.attributes)))
+        )
 
     # Emit vtable setup function
     emitter.emit_line('static bool')
@@ -398,43 +360,59 @@ def trait_vtable_name(trait: ClassIR) -> str:
     subtables = []
     for trait, vtable in base.trait_vtables.items():
         name = trait_vtable_name(trait)
+        offset_name = trait_offset_table_name(trait)
         generate_vtable(vtable, name, emitter, [], shadow)
-        subtables.append((trait, name))
+        generate_offset_table(offset_name, emitter, trait, base)
+        subtables.append((trait, name, offset_name))
 
     generate_vtable(base.vtable_entries, vtable_name, emitter, subtables, shadow)
 
     emitter.emit_line('return 1;')
     emitter.emit_line('}')
 
-    return vtable_name if not subtables else "{} + {}".format(vtable_name, len(subtables) * 2)
+    return vtable_name if not subtables else "{} + {}".format(vtable_name, len(subtables) * 3)
+
+
+def generate_offset_table(trait_offset_table_name: str,
+                          emitter: Emitter,
+                          trait: ClassIR,
+                          cl: ClassIR) -> None:
+    """Generate attribute offset row of a trait vtable."""
+    emitter.emit_line('size_t {}_scratch[] = {{'.format(trait_offset_table_name))
+    for attr in trait.attributes:
+        emitter.emit_line('offsetof({}, {}),'.format(
+            cl.struct_name(emitter.names), emitter.attr(attr)
+        ))
+    if not trait.attributes:
+        # This is for msvc.
+        emitter.emit_line('0')
+    emitter.emit_line('};')
+    emitter.emit_line('memcpy({name}, {name}_scratch, sizeof({name}));'.format(
+        name=trait_offset_table_name)
+    )
 
 
 def generate_vtable(entries: VTableEntries,
                     vtable_name: str,
                     emitter: Emitter,
-                    subtables: List[Tuple[ClassIR, str]],
+                    subtables: List[Tuple[ClassIR, str, str]],
                     shadow: bool) -> None:
     emitter.emit_line('CPyVTableItem {}_scratch[] = {{'.format(vtable_name))
     if subtables:
         emitter.emit_line('/* Array of trait vtables */')
-        for trait, table in subtables:
-            emitter.emit_line('(CPyVTableItem){}, (CPyVTableItem){},'.format(
-                emitter.type_struct_name(trait), table))
+        for trait, table, offset_table in subtables:
+            emitter.emit_line(
+                '(CPyVTableItem){}, (CPyVTableItem){}, (CPyVTableItem){},'.format(
+                    emitter.type_struct_name(trait), table, offset_table))
         emitter.emit_line('/* Start of real vtable */')
 
     for entry in entries:
-        if isinstance(entry, VTableMethod):
-            method = entry.shadow_method if shadow and entry.shadow_method else entry.method
-            emitter.emit_line('(CPyVTableItem){}{}{},'.format(
-                emitter.get_group_prefix(entry.method.decl),
-                NATIVE_PREFIX,
-                method.cname(emitter.names)))
-        else:
-            cl, attr, is_setter = entry
-            namer = native_setter_name if is_setter else native_getter_name
-            emitter.emit_line('(CPyVTableItem){}{},'.format(
-                emitter.get_group_prefix(cl),
-                namer(cl, attr, emitter.names)))
+        method = entry.shadow_method if shadow and entry.shadow_method else entry.method
+        emitter.emit_line('(CPyVTableItem){}{}{},'.format(
+            emitter.get_group_prefix(entry.method.decl),
+            NATIVE_PREFIX,
+            method.cname(emitter.names)))
+
     # msvc doesn't allow empty arrays; maybe allowing them at all is an extension?
     if not entries:
         emitter.emit_line('NULL')
@@ -747,7 +725,8 @@ def generate_getter(cl: ClassIR,
     emitter.emit_line('{}({} *self, void *closure)'.format(getter_name(cl, attr, emitter.names),
                                                            cl.struct_name(emitter.names)))
     emitter.emit_line('{')
-    emit_undefined_check(rtype, emitter, attr_field, '==')
+    attr_expr = 'self->{}'.format(attr_field)
+    emitter.emit_undefined_attr_check(rtype, attr_expr, '==', unlikely=True)
     emitter.emit_line('PyErr_SetString(PyExc_AttributeError,')
     emitter.emit_line('    "attribute {} of {} undefined");'.format(repr(attr),
                                                                     repr(cl.name)))
@@ -770,7 +749,8 @@ def generate_setter(cl: ClassIR,
         cl.struct_name(emitter.names)))
     emitter.emit_line('{')
     if rtype.is_refcounted:
-        emit_undefined_check(rtype, emitter, attr_field, '!=')
+        attr_expr = 'self->{}'.format(attr_field)
+        emitter.emit_undefined_attr_check(rtype, attr_expr, '!=')
         emitter.emit_dec_ref('self->{}'.format(attr_field), rtype)
         emitter.emit_line('}')
     emitter.emit_line('if (value != NULL) {')
@@ -833,15 +813,3 @@ def generate_property_setter(cl: ClassIR,
                           func_ir.cname(emitter.names)))
     emitter.emit_line('return 0;')
     emitter.emit_line('}')
-
-
-def emit_undefined_check(rtype: RType, emitter: Emitter, attr: str, compare: str) -> None:
-    if isinstance(rtype, RTuple):
-        attr_expr = 'self->{}'.format(attr)
-        emitter.emit_line(
-            'if ({}) {{'.format(
-                emitter.tuple_undefined_check_cond(
-                    rtype, attr_expr, emitter.c_undefined_value, compare)))
-    else:
-        emitter.emit_line(
-            'if (self->{} {} {}) {{'.format(attr, compare, emitter.c_undefined_value(rtype)))