diff --git a/docs/SPEC.md b/docs/SPEC.md index bc7f48e5c..031512edb 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -277,6 +277,12 @@ All in `PM/commonTest`, so they run wherever the interpreter loads. other Kotlin object. `Status: implemented` — `PM/commonTest/.../pythonx/PythonxPropertyTest.kt`, `ksp-fixtures/compose/.../PythonContentRenderTest.kt`, `AnySlotScalarRenderTest.kt`, `MaterialIconsRenderTest.kt`. +- **U-11** A KSP-bound function, method or property whose declared return type is a Kotlin class that + has a generated proxy returns an instance of **that class's proxy** (#94), not a generic owner: its + methods and properties work and `isinstance(result, TheClass)` holds. The class is found by the declared + return type's Kotlin name; a return type with no generated proxy stays a generic owner object. The + result's ownership is the same as for any owned result. `Status: implemented` — + `ksp-fixtures/app/.../KspClassResultProxyTest.kt` (red until #94 lands). ## 6. Binding prebuilt libraries (Gradle plugin) diff --git a/ksp-fixtures/app/src/desktopTest/kotlin/fixture/app/KspClassResultProxyTest.kt b/ksp-fixtures/app/src/desktopTest/kotlin/fixture/app/KspClassResultProxyTest.kt new file mode 100644 index 000000000..5c3881ec7 --- /dev/null +++ b/ksp-fixtures/app/src/desktopTest/kotlin/fixture/app/KspClassResultProxyTest.kt @@ -0,0 +1,80 @@ +package fixture.app + +import python.multiplatform.ffi.Python3 +import python.multiplatform.ffi.upcall.PythonProxySource +import python.multiplatform.ffi.upcall.UpcallBootstrap +import python.multiplatform.generated.FunctionTable +import python.multiplatform.reflection.HandleTable +import python.multiplatform.reflection.UpcallTable +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.Test + +/** + * #94 (SPEC U-11): a KSP function or method returning a Kotlin class gives that class's rendered + * proxy, not a generic `_PmObject`. Ownership is not what is under test here. + * + * Red before the fix: `isinstance(_c, Counter)` is False and `_c.increment` raises + * `AttributeError: '_PmObject' object has no attribute 'increment'`. + */ +class KspClassResultProxyTest { + + @BeforeTest + fun install() { + Python3.initialize(silent = true) + UpcallTable.clear() + HandleTable.releaseAll() + UpcallTable.install(FunctionTable.fragments) + check(UpcallBootstrap.publishToGlobals()) { "UpcallBootstrap.publishToGlobals() failed" } + PythonProxySource.install() + } + + @AfterTest + fun cleanup() { + Python3.exec("_c = None\n_t = None\nimport gc\ngc.collect()") + UpcallTable.clear() + HandleTable.releaseAll() + } + + @Test + fun aTopLevelFunctionReturningAClassGivesTheClassProxy() { + Python3.exec( + """ + from fixture.library import Counter, makeCounter + _c = makeCounter(5) + assert isinstance(_c, Counter), 'expected a Counter proxy, got ' + repr(type(_c)) + assert _c.increment(2) == 7, 'method call on the result failed' + assert _c.count == 7, 'property read on the result failed; got ' + repr(_c.count) + assert _c.label('n=') == 'n=7' + """.trimIndent(), + ) + } + + @Test + fun aMethodReturningAClassGivesTheClassProxy() { + Python3.exec( + """ + from fixture.library import Counter + _c = Counter(4) + _t = _c.twin() + assert isinstance(_t, Counter), 'expected a Counter proxy, got ' + repr(type(_t)) + assert _t is not _c + assert _t.increment(1) == 5 + assert _c.count == 4, 'the twin must be a separate Kotlin object' + """.trimIndent(), + ) + } + + /** A result is itself a proxy, so a method on it can return another. */ + @Test + fun aReturnedProxyCanBeChained() { + Python3.exec( + """ + from fixture.library import makeCounter + _c = makeCounter(1) + _c2 = _c.twin() + assert _c2.count == 1 + """.trimIndent(), + ) + } +} diff --git a/ksp-fixtures/library/src/commonMain/kotlin/fixture/library/LibFunctions.kt b/ksp-fixtures/library/src/commonMain/kotlin/fixture/library/LibFunctions.kt index ae1e5b17d..803240116 100644 --- a/ksp-fixtures/library/src/commonMain/kotlin/fixture/library/LibFunctions.kt +++ b/ksp-fixtures/library/src/commonMain/kotlin/fixture/library/LibFunctions.kt @@ -24,10 +24,16 @@ class Counter(var count: Long = 0) { fun label(prefix: String): String = "$prefix$count" + /** A method returning a Kotlin class (#94): the result must be a `Counter` proxy. */ + fun twin(): Counter = Counter(count) + @PythonInternal fun hiddenMethod(): Long = -1L } +/** A top-level function returning a Kotlin class (#94): the result must be a `Counter` proxy. */ +fun makeCounter(start: Long): Counter = Counter(start) + /** Holds Python references directly, the way a real exposed class would -- exercises the * generator's `tp_traverse` field-detection path (docs/design/object-lifetime.md). */ class RefHolder(var primary: PyObject?, var secondary: PyObject?) diff --git a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt index 88599adb6..b17fd4184 100644 --- a/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt +++ b/python-multiplatform/src/commonMain/kotlin/python/multiplatform/ffi/upcall/PythonProxySource.kt @@ -245,15 +245,14 @@ import python.multiplatform.reflection.UpcallTable * * **It is gated on the producer having named the return type**, and [ownedTypeOf] is where the * reason is written out: `OBJECT` means two different things in the result direction and the tag - * alone cannot separate them. The walker names its return types; KSP does not, so the KSP path - * keeps the bare-handle contract exactly as [ProxyHandleLifetimeTest] pins it. + * alone cannot separate them. The walker and KSP both name their return types, and a name that matches a + * rendered class is wrapped in that class (`_pm_classes`, #94); any other is a generic `_PmObject`. * * ### What is still not rendered * * | kind | why not | * |---|---| - * | a `TypeTag.OBJECT` result wrapped in the **class rendered for its type** | an owned result is a generic `_PmObject`, not a `Counter`. Nothing would be gained today: the walker emits no `ReflectedClass` at all (`ArtifactScanner` records constructors as needing one "which is the next step"), so no rendered class has ever shared a name with a walked return type. `docs/design/kotlin-extensions-in-python.md` §4.1's per-receiver proxy is where that belongs, and `_pm_type` carries the Kotlin type name so it has something to key on | - * | a `TypeTag.OBJECT` result read through a **module attribute** (a top-level or `object` property) | `_pm_static_property` is one shared descriptor for every module attribute and does not see the entry, so owning there means either a `_pm_own` call on the read path of *every* top-level `val` -- a row `GeneratedProxyCostTest` measures at 6-14 ns, which this would multiply -- or a second copy of the descriptor. Neither is worth building for a case no producer reaches: KSP emits no return type name, and the walker emits no properties | + * | a `TypeTag.OBJECT` result read through a **module attribute** (a top-level or `object` property) | `_pm_static_property` is one shared descriptor for every module attribute and does not see the entry, so owning there means either a `_pm_own` call on the read path of *every* top-level `val` -- a row `GeneratedProxyCostTest` measures at 6-14 ns, which this would multiply -- or a second copy of the descriptor. Neither is worth building for a case no producer reaches: the walker emits no properties, and a KSP top-level `val` of a class type is not wrapped here | * * These are skipped silently *here* because the skip is a property of this stage, not a policy * decision -- `docs/design/binding-policy.md` already decided they are exposed, and they remain reachable @@ -756,12 +755,27 @@ object PythonProxySource { self._pm_handle = None _pm_r(_pm_h) + # Kotlin type name -> the class `renderClass` rendered for it. Filled by each rendered + # class's own registration line, so a reinstall replaces an entry and never duplicates it. + _pm_classes = {} + def _pm_own(_pm_h, _pm_t=None): # `None` is how a nullable Kotlin return arrives -- `marshalResult` answers a null # with Python's `None` whatever the tag says -- and there is nothing to own. if _pm_h is None: return None - return _PmObject(_pm_h, _pm_t) + # The class rendered for the declared type, when there is one: the instance is built + # without `__init__` (which would call the Kotlin constructor) and gets the handle + # exactly where a constructed one keeps it. Ownership is unchanged -- the same + # owner base releases it. A type nothing rendered (a walked class, a library type) + # stays a generic `_PmObject`. + _pm_c = _pm_classes.get(_pm_t) + if _pm_c is None: + return _PmObject(_pm_h, _pm_t) + _pm_o = _pm_c.__new__(_pm_c) + _pm_o._pm_handle = _pm_h + _pm_o._pm_type = _pm_t + return _pm_o def _pm_owned_new(_pm_m, _pm_n, _pm_b, _pm_ns, **_pm_kw): # How a rendered class that has a **metaclass** becomes an owner. A rendered class @@ -922,10 +936,10 @@ object PythonProxySource { * So the gate is [ExposedCallable.returnTypeName]: the producer naming the Kotlin type is the * only signal that says "this really is a handle". The artefact walker supplies it * (`WalkedArtifactComposeModifierTest` asserts `androidx.compose.ui.Modifier` on `padding__Dp`); - * the KSP processor supplies none at all, so every KSP entry keeps the bare-handle contract - * `ProxyHandleLifetimeTest.testRawHandleIsTheCallersToRelease` pins and nothing on that path - * moves. Teaching KSP to emit the name is what would extend this to it, and that is the - * processor's change, not this one's. + * the KSP processor supplies it too (`FragmentScanner` fills `returnTypeName` for + * functions, methods and properties from the declared return type), so a KSP result is owned + * the same way. `_pm_own` then answers with the class rendered for that name when there is one + * (#94), and with a generic `_PmObject` otherwise. */ private fun ownedTypeOf(entry: ExposedCallable): String? = entry.returnTypeName?.takeIf { entry.returnType == TypeTag.OBJECT && it !in NOT_A_HANDLE } @@ -1238,6 +1252,8 @@ object PythonProxySource { appendLine() appendLine("$className.__qualname__ = ${cls.name.quoted()}") appendLine("setattr(_pm_module(${module.quoted()}), ${className.quoted()}, $className)") + // The lookup `_pm_own` uses to hand a result back as this class, keyed by the Kotlin name. + appendLine("_pm_classes[${cls.name.quoted()}] = $className") } return source to index }