Improve error message when trying to delete builtin

[sergey-miryanov]

Feature or enhancement

Proposal:

Following code produces not very clear error message (IMHO)

>>> all
<built-in function all>
>>> all in globals()
False
>>> del all
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    del all
        ^^^
NameError: name 'all' is not defined

On my sight it well be better to tell that builtin cannot be deleted.

(fixed can be to cannot be)

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

• gh-138890: Add clearer message when deleting builtins with del #139078

[Tapeline]

We could add a clearer error message, e.g.:

>>> del all
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    del all
        ^^^
NameError: cannot delete builtin 'all'

[StanFromIreland]

Prior art: in b86ed8e a range of cases was covered, e.g.:

>>> del 11
  File "<python-input-2>", line 1
    del 11
        ^^
SyntaxError: cannot delete literal

[sergey-miryanov]

Hm, I'm not sure it should be handled while parsing. Or I don't get what you mean.

[StanFromIreland]

It is currently handled in the parser,

cpython/Parser/pegen.h

Lines 238 to 258 in 8b5ce31

	Py_LOCAL_INLINE(void *)
	_RAISE_SYNTAX_ERROR_INVALID_TARGET(Parser *p, TARGETS_TYPE type, void *e)
	{
	expr_ty invalid_target = CHECK_NULL_ALLOWED(expr_ty, _PyPegen_get_invalid_target(e, type));
	if (invalid_target != NULL) {
	const char *msg;
	if (type == STAR_TARGETS || type == FOR_TARGETS) {
	msg = "cannot assign to %s";
	}
	else {
	msg = "cannot delete %s";
	}
	return RAISE_SYNTAX_ERROR_KNOWN_LOCATION(
	invalid_target,
	msg,
	_PyPegen_get_expr_name(invalid_target)
	);
	return RAISE_SYNTAX_ERROR_KNOWN_LOCATION(invalid_target, "invalid syntax");
	}
	return NULL;
	}

[sergey-miryanov]

Why are you talking about literals? Builtin all is not a literal. And following is a valid Python code:

all = 'xxx'
del all

So all is not an invalid target. Or I still not get you, sorry.

[StanFromIreland]

I am talking about the existing improved cases.

[sergey-miryanov]

Ough, ok. Thanks!

[StanFromIreland]

Anyhow, a quick patch:

Subject: [PATCH] Commit
---
Index: Python/codegen.c
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/Python/codegen.c b/Python/codegen.c
--- a/Python/codegen.c	(revision 8b5ce31c2b44d9bf82e6119e90a52dd530bfd1db)
+++ b/Python/codegen.c	(date 1757858640025)
@@ -3278,7 +3278,14 @@
                 : LOAD_NAME;
             break;
         case Store: op = STORE_NAME; break;
-        case Del: op = DELETE_NAME; break;
+        case Del:
+            PyObject *builtins = PyEval_GetBuiltins();
+            if (PyDict_Contains(builtins, mangled)) {
+                _PyCompile_Error(c, loc, "cannot delete builtin");
+                goto error;
+            }
+            op = DELETE_NAME;
+            break;
         }
         break;
     }

Though I worry about performance cost.

[sobolevn]

I think that we can't do that in compiler, because users can shadow buitlins:

>>> all = True
>>> del all

This must continue to work. DELETE_NAME is the most probable place for this.

[Tapeline]

Maybe the best place to put this check is in err ≠ 0 block of DELETE_NAME instruction:

PyObject *name = GETITEM(FRAME_CO_NAMES, oparg);
PyObject *ns = LOCALS();
int err;
if (ns == NULL) {
    _PyErr_Format(tstate, PyExc_SystemError,
                  "no locals when deleting %R", name);
    ERROR_NO_POP();
}
err = PyObject_DelItem(ns, name);
// Can't use ERROR_IF here.
if (err != 0) {
    // if name in builtins -> cannot delete builtin
    // else ->
    _PyEval_FormatExcCheckArg(tstate, PyExc_NameError,
                              NAME_ERROR_MSG,
                              name);
    ERROR_NO_POP();
}

[sergey-miryanov]

You also have to do same check in DELETE_GLOBAL and DELETE_FAST.

[Tapeline]

I'd like to solve this issue :)

[sergey-miryanov]

Yeah, go ahead. Feel free to ask if you have some questions!