[mypyc] Only allow deleting attributes declared using __deletable__ (#…

…10524)

Native classes now must declare attributes that can be deleted.

This improves type safety and will make it possible to support always defined 
attributes in the future. 

Example:

```
class C:
    x: int
    y: int
    z: int
    __deletable__ = ['x', 'y']
```

Now `x` and `y` can be deleted, but `z` can't be:

```
def f(c: C) -> None:
    del c.x  # Ok
    del c.y  # Ok
    del c.z  # Error
```

`__deletable__` has no effect in non-native classes and their behavior is same
 as before.

Closes mypyc/mypyc#853.

mypy/checker.py

@@ -1731,6 +1731,7 @@ def visit_class_def(self, defn: ClassDef) -> None:
             if not defn.has_incompatible_baseclass:
                 # Otherwise we've already found errors; more errors are not useful
                 self.check_multiple_inheritance(typ)
+            self.check_final_deletable(typ)
 
             if defn.decorators:
                 sig = type_object_type(defn.info, self.named_type)  # type: Type
@@ -1757,6 +1758,14 @@ def visit_class_def(self, defn: ClassDef) -> None:
         if typ.is_protocol and typ.defn.type_vars:
             self.check_protocol_variance(defn)
 
+    def check_final_deletable(self, typ: TypeInfo) -> None:
+        # These checks are only for mypyc. Only perform some checks that are easier
+        # to implement here than in mypyc.
+        for attr in typ.deletable_attributes:
+            node = typ.names.get(attr)
+            if node and isinstance(node.node, Var) and node.node.is_final:
+                self.fail(message_registry.CANNOT_MAKE_DELETABLE_FINAL, node.node)
+
     def check_init_subclass(self, defn: ClassDef) -> None:
         """Check that keywords in a class definition are valid arguments for __init_subclass__().
 
@@ -1927,8 +1936,8 @@ class C(B, A[int]): ...  # this is unsafe because...
             self.msg.cant_override_final(name, base2.name, ctx)
         if is_final_node(first.node):
             self.check_if_final_var_override_writable(name, second.node, ctx)
-        # __slots__ is special and the type can vary across class hierarchy.
-        if name == '__slots__':
+        # __slots__ and __deletable__ are special and the type can vary across class hierarchy.
+        if name in ('__slots__', '__deletable__'):
             ok = True
         if not ok:
             self.msg.base_class_definitions_incompatible(name, base1, base2,
@@ -2236,6 +2245,9 @@ def check_compatibility_all_supers(self, lvalue: RefExpr, lvalue_type: Optional[
                 # redefine it.
                 if lvalue_node.name == "__slots__" and base.fullname != "builtins.object":
                     continue
+                # We don't care about the type of "__deletable__".
+                if lvalue_node.name == "__deletable__":
+                    continue
 
                 if is_private(lvalue_node.name):
                     continue

mypy/message_registry.py

@@ -129,6 +129,8 @@
     "Final name declared in class body cannot depend on type variables"  # type: Final
 CANNOT_ACCESS_FINAL_INSTANCE_ATTR = \
     'Cannot access final instance attribute "{}" on class object'  # type: Final
+CANNOT_MAKE_DELETABLE_FINAL = \
+    "Deletable attribute cannot be final"  # type: Final
 
 # ClassVar
 CANNOT_OVERRIDE_INSTANCE_VAR = \

mypy/nodes.py

@@ -608,6 +608,8 @@ class FuncItem(FuncBase):
                  'expanded',  # Variants of function with type variables with values expanded
                  )
 
+    __deletable__ = ('arguments', 'max_pos', 'min_args')
+
     def __init__(self,
                  arguments: List[Argument],
                  body: 'Block',
@@ -2348,6 +2350,7 @@ class is generic then it will be a type constructor of higher kind.
     is_protocol = False                    # Is this a protocol class?
     runtime_protocol = False               # Does this protocol support isinstance checks?
     abstract_attributes = None  # type: List[str]
+    deletable_attributes = None  # type: List[str]  # Used by mypyc only
 
     # The attributes 'assuming' and 'assuming_proper' represent structural subtype matrices.
     #
@@ -2450,6 +2453,7 @@ def __init__(self, names: 'SymbolTable', defn: ClassDef, module_name: str) -> No
         self._fullname = defn.fullname
         self.is_abstract = False
         self.abstract_attributes = []
+        self.deletable_attributes = []
         self.assuming = []
         self.assuming_proper = []
         self.inferring = []

mypy/semanal.py

@@ -156,6 +156,8 @@ class SemanticAnalyzer(NodeVisitor[None],
     AST. Note that type checking is performed as a separate pass.
     """
 
+    __deletable__ = ['patches', 'options', 'cur_mod_node']
+
     # Module name space
     modules = None  # type: Dict[str, MypyFile]
     # Global name space for current module
@@ -2025,6 +2027,7 @@ def visit_assignment_stmt(self, s: AssignmentStmt) -> None:
         if not s.type:
             self.process_module_assignment(s.lvalues, s.rvalue, s)
         self.process__all__(s)
+        self.process__deletable__(s)
 
     def analyze_identity_global_assignment(self, s: AssignmentStmt) -> bool:
         """Special case 'X = X' in global scope.
@@ -3314,6 +3317,25 @@ def process__all__(self, s: AssignmentStmt) -> None:
                 isinstance(s.rvalue, (ListExpr, TupleExpr))):
             self.add_exports(s.rvalue.items)
 
+    def process__deletable__(self, s: AssignmentStmt) -> None:
+        if not self.options.mypyc:
+            return
+        if (len(s.lvalues) == 1 and isinstance(s.lvalues[0], NameExpr) and
+                s.lvalues[0].name == '__deletable__' and s.lvalues[0].kind == MDEF):
+            rvalue = s.rvalue
+            if not isinstance(rvalue, (ListExpr, TupleExpr)):
+                self.fail('"__deletable__" must be initialized with a list or tuple expression', s)
+                return
+            items = rvalue.items
+            attrs = []
+            for item in items:
+                if not isinstance(item, StrExpr):
+                    self.fail('Invalid "__deletable__" item; string literal expected', item)
+                else:
+                    attrs.append(item.value)
+            assert self.type
+            self.type.deletable_attributes = attrs
+
     #
     # Misc statements
     #

mypyc/codegen/emitclass.py

@@ -818,12 +818,24 @@ def generate_setter(cl: ClassIR,
         setter_name(cl, attr, emitter.names),
         cl.struct_name(emitter.names)))
     emitter.emit_line('{')
+
+    deletable = cl.is_deletable(attr)
+    if not deletable:
+        emitter.emit_line('if (value == NULL) {')
+        emitter.emit_line('PyErr_SetString(PyExc_AttributeError,')
+        emitter.emit_line('    "{} object attribute {} cannot be deleted");'.format(repr(cl.name),
+                                                                                    repr(attr)))
+        emitter.emit_line('return -1;')
+        emitter.emit_line('}')
+
     if rtype.is_refcounted:
         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) {')
+
+    if deletable:
+        emitter.emit_line('if (value != NULL) {')
     if rtype.is_unboxed:
         emitter.emit_unbox('value', 'tmp', rtype, custom_failure='return -1;', declare_dest=True)
     elif is_same_type(rtype, object_rprimitive):
@@ -834,8 +846,10 @@ def generate_setter(cl: ClassIR,
                            '    return -1;')
     emitter.emit_inc_ref('tmp', rtype)
     emitter.emit_line('self->{} = tmp;'.format(attr_field))
-    emitter.emit_line('} else')
-    emitter.emit_line('    self->{} = {};'.format(attr_field, emitter.c_undefined_value(rtype)))
+    if deletable:
+        emitter.emit_line('} else')
+        emitter.emit_line('    self->{} = {};'.format(attr_field,
+                                                      emitter.c_undefined_value(rtype)))
     emitter.emit_line('return 0;')
     emitter.emit_line('}')
 

mypyc/doc/native_classes.rst

@@ -159,6 +159,39 @@ as pure native classes.
    If a class definition uses an unsupported class decorator, *mypyc
    compiles the class into a regular Python class*.
 
+Deleting attributes
+-------------------
+
+By default, attributes defined in native classes can't be deleted. You
+can explicitly allow certain attributes to be deleted by using
+``__deletable__``::
+
+   class Cls:
+       x: int = 0
+       y: int = 0
+       other: int = 0
+
+       __deletable__ = ['x', 'y']  # 'x' and 'y' can be deleted
+
+   o = Cls()
+   del o.x  # OK
+   del o.y  # OK
+   del o.other  # Error
+
+You must initialize the ``__deletable__`` attribute in the class body,
+using a list or a tuple expression with only string literal items that
+refer to attributes. These are not valid::
+
+   a = ['x', 'y']
+
+   class Cls:
+       x: int
+       y: int
+
+       __deletable__ = a  # Error: cannot use variable 'a'
+
+   __deletable__ = ('a',)  # Error: not in a class body
+
 Other properties
 ----------------
 

mypyc/errors.py

@@ -13,6 +13,9 @@ def error(self, msg: str, path: str, line: int) -> None:
         self._errors.report(line, None, msg, severity='error', file=path)
         self.num_errors += 1
 
+    def note(self, msg: str, path: str, line: int) -> None:
+        self._errors.report(line, None, msg, severity='note', file=path)
+
     def warning(self, msg: str, path: str, line: int) -> None:
         self._errors.report(line, None, msg, severity='warning', file=path)
         self.num_warnings += 1

mypyc/ir/class_ir.py

@@ -110,6 +110,8 @@ def __init__(self, name: str, module_name: str, is_trait: bool = False,
         self.ctor = FuncDecl(name, None, module_name, FuncSignature([], RInstance(self)))
 
         self.attributes = OrderedDict()  # type: OrderedDict[str, RType]
+        # Deletable attributes
+        self.deletable = []  # type: List[str]
         # We populate method_types with the signatures of every method before
         # we generate methods, and we rely on this information being present.
         self.method_decls = OrderedDict()  # type: OrderedDict[str, FuncDecl]
@@ -211,6 +213,12 @@ def has_attr(self, name: str) -> bool:
             return False
         return True
 
+    def is_deletable(self, name: str) -> bool:
+        for ir in self.mro:
+            if name in ir.deletable:
+                return True
+        return False
+
     def name_prefix(self, names: NameGenerator) -> str:
         return names.private_name(self.module_name, self.name)
 

mypyc/irbuild/builder.py

@@ -1119,6 +1119,9 @@ def warning(self, msg: str, line: int) -> None:
     def error(self, msg: str, line: int) -> None:
         self.errors.error(msg, self.module_path, line)
 
+    def note(self, msg: str, line: int) -> None:
+        self.errors.note(msg, self.module_path, line)
+
 
 def gen_arg_defaults(builder: IRBuilder) -> None:
     """Generate blocks for arguments that have default values.

mypyc/irbuild/classdef.py

@@ -362,7 +362,12 @@ def generate_attr_defaults(builder: IRBuilder, cdef: ClassDef) -> None:
                     and isinstance(stmt.lvalues[0], NameExpr)
                     and not is_class_var(stmt.lvalues[0])
                     and not isinstance(stmt.rvalue, TempNode)):
-                if stmt.lvalues[0].name == '__slots__':
+                name = stmt.lvalues[0].name
+                if name == '__slots__':
+                    continue
+
+                if name == '__deletable__':
+                    check_deletable_declaration(builder, cls, stmt.line)
                     continue
 
                 # Skip type annotated assignments in dataclasses
@@ -398,6 +403,22 @@ def generate_attr_defaults(builder: IRBuilder, cdef: ClassDef) -> None:
     builder.leave_method()
 
 
+def check_deletable_declaration(builder: IRBuilder, cl: ClassIR, line: int) -> None:
+    for attr in cl.deletable:
+        if attr not in cl.attributes:
+            if not cl.has_attr(attr):
+                builder.error('Attribute "{}" not defined'.format(attr), line)
+                continue
+            for base in cl.mro:
+                if attr in base.property_types:
+                    builder.error('Cannot make property "{}" deletable'.format(attr), line)
+                    break
+            else:
+                _, base = cl.attr_details(attr)
+                builder.error(('Attribute "{}" not defined in "{}" ' +
+                               '(defined in "{}")').format(attr, cl.name, base.name), line)
+
+
 def create_ne_from_eq(builder: IRBuilder, cdef: ClassDef) -> None:
     """Create a "__ne__" method from a "__eq__" method (if only latter exists)."""
     cls = builder.mapper.type_to_ir[cdef.info]

mypyc/irbuild/prepare.py

@@ -54,6 +54,8 @@ def build_type_map(mapper: Mapper,
         class_ir = ClassIR(cdef.name, module.fullname, is_trait(cdef),
                            is_abstract=cdef.info.is_abstract)
         class_ir.is_ext_class = is_extension_class(cdef)
+        if class_ir.is_ext_class:
+            class_ir.deletable = cdef.info.deletable_attributes[:]
         # If global optimizations are disabled, turn of tracking of class children
         if not options.global_opts:
             class_ir.children = None
@@ -179,7 +181,7 @@ def prepare_class_def(path: str, module_name: str, cdef: ClassDef,
 
         if isinstance(node.node, Var):
             assert node.node.type, "Class member %s missing type" % name
-            if not node.node.is_classvar and name != '__slots__':
+            if not node.node.is_classvar and name not in ('__slots__', '__deletable__'):
                 ir.attributes[name] = mapper.type_to_rtype(node.node.type)
         elif isinstance(node.node, (FuncDef, Decorator)):
             prepare_method_def(ir, module_name, cdef, mapper, node.node)

mypyc/irbuild/statement.py

@@ -20,7 +20,7 @@
     Assign, Unreachable, RaiseStandardError, LoadErrorValue, BasicBlock, TupleGet, Value, Register,
     Branch, NO_TRACEBACK_LINE_NO
 )
-from mypyc.ir.rtypes import exc_rtuple
+from mypyc.ir.rtypes import RInstance, exc_rtuple
 from mypyc.primitives.generic_ops import py_delattr_op
 from mypyc.primitives.misc_ops import type_op
 from mypyc.primitives.exc_ops import (
@@ -658,6 +658,13 @@ def transform_del_item(builder: IRBuilder, target: AssignmentTarget, line: int)
             line=line
         )
     elif isinstance(target, AssignmentTargetAttr):
+        if isinstance(target.obj_type, RInstance):
+            cl = target.obj_type.class_ir
+            if not cl.is_deletable(target.attr):
+                builder.error('"{}" cannot be deleted'.format(target.attr), line)
+                builder.note(
+                    'Using "__deletable__ = ' +
+                    '[\'<attr>\']" in the class body enables "del obj.<attr>"', line)
         key = builder.load_str(target.attr)
         builder.call_c(py_delattr_op, [target.obj, key], line)
     elif isinstance(target, AssignmentTargetRegister):

mypyc/test-data/irbuild-classes.test

@@ -1069,3 +1069,68 @@ class A(B): pass
 class B(C): pass
 class C: pass
 [out]
+
+[case testDeletableSemanticAnalysis]
+class Err1:
+    __deletable__ = 'x'  # E: "__deletable__" must be initialized with a list or tuple expression
+class Err2:
+    __deletable__ = [
+        1  # E: Invalid "__deletable__" item; string literal expected
+    ]
+class Err3:
+    __deletable__ = ['x', ['y'], 'z']  # E: Invalid "__deletable__" item; string literal expected
+class Err4:
+    __deletable__ = (1,)  # E: Invalid "__deletable__" item; string literal expected
+a = ['x']
+class Err5:
+    __deletable__ = a  # E: "__deletable__" must be initialized with a list or tuple expression
+
+class Ok1:
+    __deletable__ = ('x',)
+    x: int
+class Ok2:
+    __deletable__ = ['x']
+    x: int
+
+[case testInvalidDeletableAttribute]
+class NotDeletable:
+    __deletable__ = ['x']
+    x: int
+    y: int
+
+def g(o: NotDeletable) -> None:
+    del o.x
+    del o.y  # E: "y" cannot be deleted \
+             # N: Using "__deletable__ = ['<attr>']" in the class body enables "del obj.<attr>"
+
+class Base:
+    x: int
+
+class Deriv(Base):
+    __deletable__ = ['x']  # E: Attribute "x" not defined in "Deriv" (defined in "Base")
+
+class UndefinedDeletable:
+    __deletable__ = ['x']  # E: Attribute "x" not defined
+
+class DeletableProperty:
+    __deletable__ = ['prop']  # E: Cannot make property "prop" deletable
+
+    @property
+    def prop(self) -> int:
+        return 5
+
+[case testFinalDeletable]
+from typing import Final
+
+class DeletableFinal1:
+    x: Final[int]  # E: Deletable attribute cannot be final
+
+    __deletable__ = ['x']
+
+    def __init__(self, x: int) -> None:
+        self.x = x
+
+class DeletableFinal2:
+    X: Final = 0  # E: Deletable attribute cannot be final
+
+    __deletable__ = ['X']

mypyc/test-data/irbuild-statements.test

@@ -880,6 +880,7 @@ L0:
 
 [case testDelAttribute]
 class Dummy():
+    __deletable__ = ('x', 'y')
     def __init__(self, x: int, y: int) -> None:
         self.x = x
         self.y = y