Skip to content

fix: [Python] consistent mangled names for generic abstract class members#4544

Merged
dbrattli merged 2 commits into
mainfrom
fix/python-generic-abstract-class-naming
Apr 18, 2026
Merged

fix: [Python] consistent mangled names for generic abstract class members#4544
dbrattli merged 2 commits into
mainfrom
fix/python-generic-abstract-class-naming

Conversation

@dbrattli

Copy link
Copy Markdown
Collaborator

Summary

Derived classes of generic abstract classes could not be instantiated in compiled Python due to mismatched mangled method names between the abstract stub and the override.

Root cause

Two different name-mangling paths:

  • Abstract stubs (Fable2Python.Bases.fs, InterfaceNaming.getMangledMemberName) derived the entity prefix from memb.FullName, which from the F# compiler does not include the backtick generic arity. The code also replaced . with _ but left backticks untouched.
  • Overrides in derived classes (FSharp2Fable.Util.fs, getMangledAbstractMemberName) built the prefix from the entity's own TryFullName, which does include the arity (e.g. `2), and downstream sanitization in Python.Prelude.sanitizeIdentForbiddenChars replaced both . and ` with _.

Result: for TestRunner<'A, 'B> the abstract stub was named TestRunner_get_Encode while the override was named TestRunner_2_get_Encode. Python's ABC then refused to instantiate any derived class:

TypeError: Can't instantiate abstract class PythonTestRunner without an implementation for abstract methods
  'Thoth_Json_Tests_Testing_TestRunner_get_Decode',
  'Thoth_Json_Tests_Testing_TestRunner_get_Encode',
  ...

Fix

getMangledMemberName now uses ent.FullName (entity full name with arity) and Fable.Py.Naming.cleanNameAsPyIdentifier, which replaces both . and ` with _. This matches the mangled name that derived-class overrides produce.

For non-generic abstract classes the output is unchanged.

Test plan

  • New regression test Generic abstract class can be instantiated through derived class in tests/Python/TestType.fs covering an abstract method, an abstract property, and an abstract generic mapper.
  • Full tests/Python suite passes locally (2331 tests).
  • Verified externally against Thoth.Json's Python tests (239 tests, previously broken on this branch, now green).

🤖 Generated with Claude Code

dbrattli and others added 2 commits April 18, 2026 15:33
…bers

Abstract method stubs in a generic abstract class were produced from
`memb.FullName` (which omits the backtick generic arity), while
derived-class overrides were produced from the entity's `FullName`
(which includes the arity). The two paths also used different
sanitization: the stub path replaced `.` but not `` ` ``, while the
override path replaced both. Result: Python's ABC refused to
instantiate derived classes because the abstract method names
(e.g. `TestRunner_get_Encode`) did not match the override names
(`TestRunner_2_get_Encode`).

Use `ent.FullName` plus `Fable.Py.Naming.cleanNameAsPyIdentifier` on
both sides so the two mangled names match.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Python Type Checking Results (Pyright)

Metric Value
Total errors 33
Files with errors 4
Excluded files 4
New errors ✅ No
Excluded files with errors (4 files)

These files have known type errors and are excluded from CI. Remove from pyrightconfig.ci.json as errors are fixed.

File Errors Status
temp/tests/Python/test_hash_set.py 18 Excluded
temp/tests/Python/test_applicative.py 12 Excluded
temp/tests/Python/test_nested_and_recursive_pattern.py 2 Excluded
temp/tests/Python/fable_modules/thoth_json_python/encode.py 1 Excluded

@dbrattli dbrattli merged commit 00c8542 into main Apr 18, 2026
24 checks passed
@dbrattli dbrattli deleted the fix/python-generic-abstract-class-naming branch April 18, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant