Skip to content

Allow reify types to be mutable to avoid FrozenInstanceError from attrs #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]
### Changed
* Types generated by `reify` may optionally be marked as `^:mutable` now to prevent `attrs.exceptions.FrozenInstanceError`s being thrown when mutating methods inherited from the supertype(s) are called (#1088)

## [v0.3.0]
### Added
Expand Down
7 changes: 7 additions & 0 deletions docs/concepts.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1342,6 +1342,13 @@ Reified types always implement :py:class:`basilisp.lang.interfaces.IWithMeta` an

While ``reify`` and ``deftype`` are broadly similar, ``reify`` types may not define class or static methods.

.. warning::

If a reified type is defined with a mutable "abstract" supertype (such as :external:py:class:`io.IOBase`), users may experience errors arising from the ``attrs``-generated ``__setattr__`` method for the underlying type when mutating methods are called on the resulting object.
Reified types are immutable (or "frozen" in ``attrs`` lingo) by default.
When a mutating method, such as :external:py:meth:`io.IOBase.close`, is called on the type (which may be called manually or it may be called at VM shutdown), the mutation will fail due to ``attrs`` replacing the ``__setattr__`` method on the type.
It is possible to force Basilisp to generate a mutable (non-frozen) type for reified types by applying the ``^:mutable`` metadata on the ``reify`` symbol.

.. _defrecord:

``defrecord``
Expand Down
5 changes: 3 additions & 2 deletions src/basilisp/core.lpy
Original file line number Diff line number Diff line change
Expand Up @@ -6742,9 +6742,10 @@
the body of a method should not include that parameter, as it will be supplied
automatically."
[& method-impls]
(let [{:keys [interfaces methods]} (collect-methods method-impls)]
(let [{:keys [interfaces methods]} (collect-methods method-impls)
reify-sym (with-meta 'reify* (meta (first &form)))]
(with-meta
`(reify* :implements [~@interfaces python/object]
`(~reify-sym :implements [~@interfaces python/object]
~@methods)
(meta &form))))

Expand Down
1 change: 1 addition & 0 deletions src/basilisp/lang/compiler/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -3107,6 +3107,7 @@ def _reify_ast(form: ISeq, ctx: AnalyzerContext) -> Reify:
members=vec.vector(members),
verified_abstract=type_abstractness.is_statically_verified_as_abstract,
artificially_abstract=type_abstractness.artificially_abstract_supertypes,
is_frozen=not _is_mutable(form.first),
use_weakref_slot=not type_abstractness.supertype_already_weakref,
env=ctx.get_node_env(pos=ctx.syntax_position),
)
Expand Down
2 changes: 1 addition & 1 deletion src/basilisp/lang/compiler/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -2771,7 +2771,7 @@ def _reify_to_py_ast(
),
verified_abstract=node.verified_abstract,
artificially_abstract_bases=artificially_abstract_bases,
is_frozen=True,
is_frozen=node.is_frozen,
use_slots=True,
use_weakref_slot=node.use_weakref_slot,
)
Expand Down
1 change: 1 addition & 0 deletions src/basilisp/lang/compiler/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,7 @@ class Reify(Node[SpecialForm]):
env: NodeEnv
verified_abstract: bool = False
artificially_abstract: IPersistentSet[DefTypeBase] = lset.PersistentSet.empty()
is_frozen: bool = True
use_weakref_slot: bool = True
meta: NodeMeta = None
children: Sequence[kw.Keyword] = vec.v(MEMBERS)
Expand Down
18 changes: 9 additions & 9 deletions tests/basilisp/compiler_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -4937,21 +4937,21 @@ def test_reify_disallows_extra_methods_if_not_in_a_super_type(
(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members '(:read)} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members '(:read)} io/IOBase]
(read [this v]))""",
compiler.CompilerException,
),
(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members [:read]} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members [:read]} io/IOBase]
(read [this v]))""",
compiler.CompilerException,
),
(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #py [:read]} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #py [:read]} io/IOBase]
(read [this v]))""",
compiler.CompilerException,
),
Expand All @@ -4968,7 +4968,7 @@ def test_reify_abstract_members_must_have_elements(self, lcompile: CompileFn):
lcompile(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{}} io/IOBase]
(read [this v]))
"""
)
Expand All @@ -4979,7 +4979,7 @@ def test_reify_abstract_should_not_have_namespaces(
lcompile(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{:io/read}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{:io/read}} io/IOBase]
(read [this v]))
"""
)
Expand All @@ -4997,7 +4997,7 @@ def test_reify_abstract_members_names_must_be_str_keyword_or_symbol(
lcompile(
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{1 :read}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{1 :read}} io/IOBase]
(read [this v]))
""",
)
Expand All @@ -5007,19 +5007,19 @@ def test_reify_abstract_members_names_must_be_str_keyword_or_symbol(
[
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{:read :write}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{:read :write}} io/IOBase]
(read [this n])
(write [this v]))
""",
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{read write}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{read write}} io/IOBase]
(read [this n])
(write [this v]))
""",
"""
(import* io)
(reify* :implements [^:abstract ^{:abstract-members #{"read" "write"}} io/IOBase]
(^:mutable reify* :implements [^:abstract ^{:abstract-members #{"read" "write"}} io/IOBase]
(read [this n])
(write [this v]))
""",
Expand Down