Adding Deep Immutability to 3.12 by matajoh · Pull Request #51 · mjp41/cpython

matajoh · 2025-03-05T15:14:49Z

This draft PR is to give a chance to view the diff between 3.12 and just adding the deep immutability from veronapy. The key difference is the use of the lowest bit of ob_refcnt to indicate immutability (so as to not impact object header size).

📚 Documentation preview 📚: https://cpython-previews--51.org.readthedocs.build/

Include/internal/pycore_immutability.h

Doc/c-api/typeobj.rst

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

xFrednet

My comments can be quite nit-picky, so please don't hesitate to push back on them :)

I'm not done reviewing everything yet, but from what I've seen thus far: Excellent job! It's amazing that you managed to get all of this together, well done :D
This review is still incomplete. From all I've read, excellent job!

Include/internal/pycore_runtime_init_generated.h

Include/internal/pycore_dict.h

Include/object.h

Include/internal/pycore_object.h

Objects/abstract.c

Tools/gdb/libpython.py

xFrednet

Very nicely done! This is mainly a collection of questions, which probably come from me missing some context.

Python/immutability.c

Lib/test/test_freeze.py

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

mjp41 · 2025-04-02T13:59:23Z

Python/immutability.c

+            }else if(PyDict_Contains(module_dict, name)){
+                PyObject* value = PyDict_GetItem(module_dict, name); // value.rc = x
+
+                _PyDict_SetKeyImmutable((PyDictObject*)module_dict, name);
+
+                if(!_Py_IsImmutable(value)){
+                    if(PyList_Append(frontier, value)){
+                        goto nomemory;
+                    }
+                }
+            }
+        }


I don't understand what this is doing? Could you explain an example that this is catching that is a problem?

mjp41 · 2025-04-07T11:55:09Z

Python/immutability.c

+            }else if(PyDict_Contains(module_dict, name)){
+                PyObject* value = PyDict_GetItem(module_dict, name);
+
+                _PyDict_SetKeyImmutable((PyDictObject*)module_dict, name);
+
+                if(!_Py_IsImmutable(value)){
+                    if(push(frontier, value)){
+                        goto nomemory;
+                    }
+                }
+            }


I couldn't work out how to exercise this code. It doesn't appear to be hit by test_freeze. Should we be able to get coverage for this? I had an experiment with the REPL and a few modules, but this code path didn't get exercised.

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

xFrednet · 2025-05-05T11:33:53Z

Include/internal/pycore_interp.h

    struct _Py_async_gen_state async_gen;
    struct _Py_context_state context;
    struct _Py_exc_state exc_state;
+    struct _Py_immutability_state immutability;


Does this mean that different interpreters have different views on what is freezable?

xFrednet · 2025-05-05T11:45:27Z

Include/object.h

        return;
    }
-    ob->ob_refcnt = refcnt;
+    ob->ob_refcnt = (ob->ob_refcnt & _Py_IMMUTABLE_MASK) | (refcnt & _Py_REFCNT_MASK);


Shouldn't the immutability flag always be 0 here due to the if statement above?

xFrednet · 2025-05-08T09:30:27Z

@matajoh Do we want to change the title of the PR to be ready for outside viewers? Maybe something like "Adding Deep Immutability to 3.12"

xFrednet · 2025-05-08T09:39:15Z

Modules/_elementtree.c

+    if (!Py_CHECKWRITE(op)) {            \
+        PyErr_SetObject(                 \
+            PyExc_TypeError,      \
+            (PyObject *)((op)));                       \
+        return -1;                     \
+    }


Why does this create an Error object and doesn't use PyErr_WriteToImmutable like the additions in the other files?

Also why is it called _VALIDATE_WRITABLE here and _CHECK_IS_WRITABLE above? The difference seems to be the return value, but they seem to be used in the same way.

xFrednet · 2025-05-08T09:41:37Z

Modules/_elementtree.c

-    if (type == NULL) {                                                  \
-        goto error;                                                      \
-    }                                                                    \
+#define CREATE_TYPE(module, type, spec, base) \


Is this new base parameter still needed? It seems to be always NULL in the uses below

xFrednet · 2025-05-08T09:45:41Z

Modules/_struct.c

+    if (_PyImmutability_RegisterFreezable((PyTypeObject *)state->PyStructType) < 0){
+        return -1;
+    }


The file Modules/_elementtree.c uses _PyImport_GetModuleAttrString("immutable", "register_freezable"); to get the register_freezable function or handle failure if the "immutable" module is not available. This one uses the builtin function _PyImmutability_RegisterFreezable from pycore. How is it decided which function is used?

xFrednet · 2025-05-08T09:48:35Z

Modules/cjkcodecs/multibytecodec.c

+    if(self->codec->encinit != NULL){
+        if(!Py_CHECKWRITE(self)){
+            return PyErr_WriteToImmutable(self);
+        }
+    }


This file has new Py_CHECKWRITE but doesn't register the type as freezable. Is this declared somewhere else?

xFrednet · 2025-05-08T09:50:42Z

Modules/clinic/immutablemodule.c.h

+#if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE)
+#  include "pycore_gc.h"            // PyGC_Head
+#  include "pycore_runtime.h"       // _Py_ID()
+#endif


These two includes don't seem to be used in the header file, can we move them into the .c file instead?

xFrednet · 2025-05-08T09:58:59Z

Modules/immutablemodule.c

+#include "clinic/immutablemodule.c.h"
+
+typedef struct {
+    PyObject *not_freezable_error_obj;


Is this object still used? Looking at the code, it seems to only be initialized and cleared, but never used outsite of this

xFrednet · 2025-05-08T10:05:08Z

Objects/abstract.c

+static PyObject *
+immutable_error(void)
+{
+    PyThreadState *tstate = _PyThreadState_GET();
+    if (!_PyErr_Occurred(tstate)) {
+        _PyErr_SetString(tstate, PyExc_TypeError,
+                         "cannot modify immutable instance");
+    }
+    return NULL;
+}


Why does this used a custom immutable_error function and not PyErr_WriteToImmutable(). The difference seems to be that this function doesn't take the object that caused the error. But looking at the Py_CHECKWRITE(o) we always seem to have an object

xFrednet · 2025-05-08T10:41:34Z

Python/immutability.c

+    if(PyDict_SetItemString(PyModule_GetDict(frozen_importlib), "_freezable_types", state->freezable_types)){
+        Py_DECREF(frozen_importlib);
+        return -1;
+    }


My understanding is, that this exposes the freezable_types set via the frozen_importlib module. Is there a reason for this?

xFrednet · 2025-05-08T10:47:44Z

Python/immutability.c

+    size = PySequence_Fast_GET_SIZE(f_code->co_consts);
+    for(Py_ssize_t i = 0; i < size; i++){
+        PyObject* value = PySequence_Fast_GET_ITEM(f_code->co_consts, i);
+        if(check_globals && PyUnicode_Check(value)){


Couldn't we move the check_globals check outside the for loop?

xFrednet · 2025-05-08T10:55:30Z

Python/immutability.c

+        if(_Py_IsImmutable(item)){
+            continue;
+        }


Can't we move this check above check_freezable? It seems like it's way cheaper.

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

alwaysmpe · 2025-07-02T01:13:35Z

I've been playing with the current implementation. Seems that it’s sometimes trying to freeze type variables that are arguments to generic functions. And that can’t be done. A simple decorator example:

from __future__ import annotations
from immutable import freeze
from typing import Callable
def foo[**P, R](fn: Callable[P, R]) -> Callable[P, R]:
    def fn_wrap(*args: P.args, **kwargs: P.kwargs) -> R:
        return fn(*args, **kwargs)
    return fn_wrap
freeze(foo) # TypeError: Cannot freeze instance of type typing.ParamSpec due to custom functionality implemented in C

I’m surprised that from __future__ import annotations doesn’t make a difference.

mjp41 · 2025-07-02T14:41:38Z

I've been playing with the current implementation. Seems that it’s sometimes trying to freeze type variables that are arguments to generic functions. And that can’t be done. A simple decorator example:
from __future__ import annotations
from immutable import freeze
from typing import Callable
def foo[**P, R](fn: Callable[P, R]) -> Callable[P, R]:
    def fn_wrap(*args: P.args, **kwargs: P.kwargs) -> R:
        return fn(*args, **kwargs)
    return fn_wrap
freeze(foo) # TypeError: Cannot freeze instance of type typing.ParamSpec due to custom functionality implemented in C
I’m surprised that from __future__ import annotations doesn’t make a difference.

Thanks for raising this. I'll investigate what is happening, and get back to you.

Source: <https://discuss.python.org/t/pep-7-compatible-clang-format-config/66139>

matajoh force-pushed the immutable-pep branch 2 times, most recently from dd6a77c to 469ce9e Compare March 5, 2025 15:54

matajoh mentioned this pull request Mar 5, 2025

initial TobiasWrigstad/peps#4

Merged

mjp41 changed the base branch from pyrona to v3.12.0 March 5, 2025 16:53

matajoh force-pushed the immutable-pep branch 3 times, most recently from 9d8efcd to e676ca4 Compare March 12, 2025 16:46

xFrednet reviewed Mar 19, 2025

View reviewed changes

Include/internal/pycore_immutability.h Outdated Show resolved Hide resolved

mjp41 reviewed Mar 21, 2025

View reviewed changes

Doc/c-api/typeobj.rst Show resolved Hide resolved

matajoh force-pushed the immutable-pep branch 3 times, most recently from 8ce6f94 to 295e983 Compare March 28, 2025 12:01

Proposed code changes for [PEP XXX](link).

7ad9131

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh force-pushed the immutable-pep branch from efeec29 to 7ad9131 Compare March 28, 2025 13:46

matajoh marked this pull request as ready for review March 28, 2025 13:47

xFrednet reviewed Mar 31, 2025

View reviewed changes

xFrednet approved these changes Apr 2, 2025

View reviewed changes

matajoh force-pushed the immutable-pep branch 2 times, most recently from e269f3f to 1cdd289 Compare April 3, 2025 10:22

Addressing PR comments

3b832e6

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh force-pushed the immutable-pep branch from 1cdd289 to 3b832e6 Compare April 3, 2025 10:43

matajoh added 3 commits April 3, 2025 16:33

Cleaning up the immutability code

d52b222

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

deque working

80b648c

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Adding write barriers for array

697d3b1

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh force-pushed the immutable-pep branch from d5b94d6 to 697d3b1 Compare April 4, 2025 13:28

mjp41 reviewed Apr 7, 2025

View reviewed changes

matajoh added 2 commits April 9, 2025 14:08

Cleaning up function freezing + more tests

4024677

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Immutability for blake2 + test_freeze module

9136246

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh force-pushed the immutable-pep branch from f1afe53 to 9136246 Compare April 9, 2025 13:11

Fixing some holes in object/typeobject

61a0400

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh and others added 4 commits May 1, 2025 10:59

_struct

2fab5cd

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Making freezable types a weakset

f621a0c

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

These don't need to be freezable

a31e962

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Make mutable during finalisation. (#6)

adc1319

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh force-pushed the immutable-pep branch from 356cac3 to adc1319 Compare May 1, 2025 14:35

matajoh added 4 commits May 2, 2025 11:35

Dealing with more finalisation bugs

ccb893c

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Shadow cell

30dd83f

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Removing isfreezable and adding some TODOs

57df4db

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

Using import helper

4866d04

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

xFrednet reviewed May 5, 2025

View reviewed changes

xFrednet reviewed May 8, 2025

View reviewed changes

Adding the bases tuple as discussed

0387c12

Signed-off-by: Matthew A Johnson <matjoh@microsoft.com>

matajoh changed the title ~~Code diff with just the immutability changes~~ Adding Deep Immutability to 3.12 May 13, 2025

xFrednet mentioned this pull request Jun 4, 2025

Fix check_freezable only checking object types #58

Closed

Initial simple Python module.

c867271

tage64 force-pushed the immutable-pep branch from fa425a9 to c867271 Compare July 10, 2025 14:36

Add .clang-format file.

81aebf3

Source: <https://discuss.python.org/t/pep-7-compatible-clang-format-config/66139>

Conversation

matajoh commented Mar 5, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xFrednet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xFrednet commented May 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alwaysmpe commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjp41 commented Jul 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

matajoh commented Mar 5, 2025 •

edited by github-actions bot

Loading

alwaysmpe commented Jul 2, 2025 •

edited

Loading