From d64d90955d274c81f7444c6852e996b3fab18c96 Mon Sep 17 00:00:00 2001 From: BREW <81044362+b-re-w@users.noreply.github.com> Date: Sun, 25 Oct 2026 21:31:41 +0900 Subject: [PATCH] Fix: develop CI green again: Linux -Werror on tp_np_f64, companion stub collisions, inherited overload sets in stubs TypedPython CI: vd was assigned only under __APPLE__, so GCC rejected it as unused (#296); it is now declared in that branch. Kotlin-name stubs: a companion handle typed as its own type stays a type path (no attribute that mypy reads as a variable), and an owner-typed one is written only when no nested class of that name is rendered, on the class and the module path (#322). A subclass that overrides one overload of an inherited name now renders the full set Kotlin resolution sees, in the supertype's order (CornerBasedShape.copy / createOutline, from #307's foundation.shape walk). --- .../gradle/stubs/PyiRendering.kt | 75 +++++++++++++++++-- .../gradle/stubs/InheritedOverloadStubTest.kt | 54 +++++++++++++ .../python/typedpython/runtime/tp_runtime.h | 4 +- 3 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/InheritedOverloadStubTest.kt diff --git a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt index d161568c..0460c5db 100644 --- a/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt +++ b/python-multiplatform-gradle-plugin/src/main/kotlin/python/multiplatform/gradle/stubs/PyiRendering.kt @@ -320,6 +320,8 @@ private class StubRenderer(declarations: List) { var bases: String? = null var doc: String? = null val members = mutableListOf() + /** Companion attributes (name to text), decided in [renderNode] once every nested class is known. */ + val companionAttributes = mutableListOf>() } private inner class ModuleOut(val module: String) { @@ -708,13 +710,19 @@ private class StubRenderer(declarations: List) { // The companion object, a class attribute (`Modifier.Companion`). Typed as the owner when the // companion is one (`Companion: Modifier`), else as the companion's own type. + // A stub name is a type or a value, not both: a companion typed as its own type keeps that type's + // path (signatures name it as a type), so only a companion typed as the owner becomes an attribute, + // and only if no nested class of that name is rendered (decided in renderNode). companionHandles[qualifiedName].orEmpty().forEach { d -> val name = d.simpleName - if (!isIdentifier(name) || name in node.children) { - node.members += "# $COMPANION_MARKER${d.bindingName} is not stubbed: '$name' cannot be written as an attribute here" - return@forEach + val marker = "# $COMPANION_MARKER${d.bindingName} is not stubbed" + when { + !isIdentifier(name) -> node.members += "$marker: '$name' cannot be written as an attribute here" + d.returnType.qualifiedName != qualifiedName -> + node.members += "$marker: its type ${d.returnType.qualifiedName} is a type path here, not a value" + else -> node.companionAttributes += name to + "$name: ${result(d, ctx)}\n\"\"\"$COMPANION_MARKER${d.bindingName} (${kotlinSignatureOf(d)})\"\"\"" } - node.members += "$name: ${result(d, ctx)}\n\"\"\"$COMPANION_MARKER${d.bindingName} (${kotlinSignatureOf(d)})\"\"\"" } constructorsByClass[qualifiedName]?.let { group -> @@ -768,7 +776,7 @@ private class StubRenderer(declarations: List) { group.filter { leafOf(it) == name }.forEach { node.members += methodNotStubbed(it, "'$name' cannot be written as an attribute here") } return@forEach } - val rendered = renderMethods(name, group, ctx) + val rendered = renderMethods(name, withInherited(qualifiedName, name, group), ctx) node.members += rendered stubbedMethods[name] = name } @@ -786,10 +794,49 @@ private class StubRenderer(declarations: List) { if (!isIdentifier(alias) || alias in node.children) return@forEach protocols[kotlin]?.let { node.members += "$alias: $TYPING.ClassVar[$it]" } stubbedProperties[kotlin]?.let { (getter, setter) -> node.members += renderProperty(alias, getter, setter, ctx) } - if (kotlin in stubbedMethods && alias !in memberFunctions) node.members += renderMethods(alias, memberFunctions.getValue(kotlin), ctx) + if (kotlin in stubbedMethods && alias !in memberFunctions) node.members += renderMethods(alias, withInherited(qualifiedName, kotlin, memberFunctions.getValue(kotlin)), ctx) } } + /** + * A subclass that overrides one overload of an inherited name would, as a lone `def`, hide the + * inherited overload set from a type checker (an incompatible override), while Kotlin resolution + * still sees every inherited signature. So the set rendered for [name] on [qualifiedName] is its + * own declarations first, then each signature a stub supertype declares under [name] whose + * parameter types no earlier entry already has (nearest supertype first). Only a base name + * (not a suffixed explicit key) is widened, because only it is shared across the hierarchy. + */ + private fun withInherited(qualifiedName: String, name: String, own: List): List { + if (isSuffixed(name)) return own + return orderedOverloads(qualifiedName, name, own) + } + + private fun overloadKey(d: DeclarationModel) = d.parameters.map { it.type.qualifiedName } + + /** [type]'s own declarations under [name], in the order its own stub groups them. */ + private fun ownOverloads(type: String, name: String): List = + methods[type].orEmpty().sortedBy { it.bindingName } + .filter { leafOf(it) == name || (isSuffixed(leafOf(it)) && baseOf(leafOf(it)) == name) } + + /** + * The overload set [type] renders under [name], in an order every subclass can keep: a type checker + * requires an overriding overload set to list the supertype's variants in the supertype's order. + * So it starts from the nearest stub supertype's own ordered set, puts [own]'s override in place of + * any signature it redeclares (same parameter types), and appends [own]'s other signatures. The + * same function orders every level, so a supertype and its subclasses always agree. + */ + private fun orderedOverloads(type: String, name: String, own: List): List { + val parent = supertypes[type].orEmpty() + .filter { it != type && classRefOf(it) != null } + .asSequence() + .map { orderedOverloads(it, name, ownOverloads(it, name)) } + .firstOrNull { it.isNotEmpty() } + ?: return own + val parentKeys = parent.map(::overloadKey).toSet() + return parent.map { p -> own.firstOrNull { overloadKey(it) == overloadKey(p) } ?: p } + + own.filter { overloadKey(it) !in parentKeys } + } + /** * One member function, or an overload set under its base name, as methods named [name]. A single * declaration's method carries its table key after [METHOD_MARKER]; an overload set's `@overload`s @@ -885,7 +932,15 @@ private class StubRenderer(declarations: List) { if (ref != null && classReference(owner, null, ctx) != ANY) return@forEach if (owner in moduleNames) { val moduleCtx = out(owner) - handles.forEach { moduleCtx.defs += renderDef(it, moduleCtx) } + handles.forEach { d -> + // Same rule as the class path: a companion typed as its own type keeps that type's + // name (the module renders the class `Companion`), so it is no module attribute too. + if (d.returnType.qualifiedName != owner) { + moduleCtx.defs += "# $COMPANION_MARKER${d.bindingName} is not stubbed: its type ${d.returnType.qualifiedName} is a type path here, not a value" + } else { + moduleCtx.defs += renderDef(d, moduleCtx) + } + } } else { handles.forEach { ctx.defs += "# $COMPANION_MARKER${it.bindingName} is not stubbed: ${owner.substringAfterLast('.')} has no stub class" } } @@ -906,7 +961,11 @@ private class StubRenderer(declarations: List) { node.bases?.let { into.append('(').append(it).append(')') } into.appendLine(":") into.append(indent).append(" \"\"\"").append(node.doc ?: "Kotlin: container of nested classes").appendLine("\"\"\"") - node.members.forEach { member -> + val companions = node.companionAttributes.map { (name, text) -> + if (name in node.children) "# $COMPANION_MARKER${node.qualifiedName}.$name is not stubbed: a nested class '$name' is rendered here" + else text + } + (node.members + companions).forEach { member -> member.lines().forEach { into.append(indent).append(" ").appendLine(it) } } node.children.values.forEach { renderNode(it, "$indent ", into) } diff --git a/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/InheritedOverloadStubTest.kt b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/InheritedOverloadStubTest.kt new file mode 100644 index 00000000..526785cb --- /dev/null +++ b/python-multiplatform-gradle-plugin/src/test/kotlin/python/multiplatform/gradle/stubs/InheritedOverloadStubTest.kt @@ -0,0 +1,54 @@ +package python.multiplatform.gradle.stubs + +import python.multiplatform.gradle.model.DeclarationModel +import python.multiplatform.gradle.model.DeclaredParameter +import python.multiplatform.gradle.model.KotlinTypeModel +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertTrue + +/** + * A subclass overriding one overload of an inherited name must still declare every inherited overload, + * or a type checker reports an incompatible override (CornerBasedShape.copy and RoundedCornerShape.copy). + */ +class InheritedOverloadStubTest { + + private val pkg = "fixture" + private val base = KotlinTypeModel("$pkg.Base") + private val sub = KotlinTypeModel("$pkg.Sub") + private val int = KotlinTypeModel("kotlin.Int") + private val string = KotlinTypeModel("kotlin.String") + + private fun producing(binding: String, type: KotlinTypeModel, supertypes: List) = DeclarationModel( + simpleName = binding.substringAfterLast('.'), owner = pkg, ownerIsClass = false, receiver = null, + parameters = emptyList(), returnType = type, returnBoundaryTag = "OBJECT", bindingName = binding, + returnSupertypes = supertypes, + ) + + private fun method(owner: KotlinTypeModel, binding: String, param: DeclaredParameter) = DeclarationModel( + simpleName = binding.substringAfterLast('.').substringBefore("__"), owner = owner.qualifiedName, + ownerIsClass = true, receiver = owner, receiverBoundaryTag = "OBJECT", parameters = listOf(param), + returnType = owner, returnBoundaryTag = "OBJECT", bindingName = binding, kind = "METHOD", + ) + + private val intParam = DeclaredParameter("a", int, declaresDefault = false, boundaryTag = "INT") + private val stringParam = DeclaredParameter("b", string, declaresDefault = false, boundaryTag = "STRING") + + @Test + fun aSubclassOverridingOneOverloadStillDeclaresTheInheritedOne() { + val files = renderKotlinFqnStubs( + listOf( + producing("$pkg.makeBase", base, emptyList()), + producing("$pkg.makeSub", sub, listOf("$pkg.Base")), + method(base, "$pkg.Base.copy__Int", intParam), + method(base, "$pkg.Base.copy__String", stringParam), + method(sub, "$pkg.Sub.copy__Int", intParam), + ), + ) + val body = files.getValue("fixture/__init__.pyi") + val subBody = body.substringAfter("class Sub(Base):") + val baseNameDefs = Regex("@_t\\.overload\\s+def copy\\(([^\\n]*)").findAll(subBody).map { it.groupValues[1] }.toList() + assertEquals(2, baseNameDefs.size, body) + assertTrue(baseNameDefs[0].contains("a: ") && baseNameDefs[1].contains("b: "), body) + } +} diff --git a/python-multiplatform-ksp/src/main/python/typedpython/runtime/tp_runtime.h b/python-multiplatform-ksp/src/main/python/typedpython/runtime/tp_runtime.h index 3cca1983..3ba26954 100644 --- a/python-multiplatform-ksp/src/main/python/typedpython/runtime/tp_runtime.h +++ b/python-multiplatform-ksp/src/main/python/typedpython/runtime/tp_runtime.h @@ -506,7 +506,7 @@ static inline double tp_np_mod_only(double a, double b) * across the flag clear and test (the function is also noinline). */ static TP_NOINLINE int tp_np_f64(int op, double a, double b, double *out) { - volatile double va = a, vb = b, vr = 0.0, vd = 0.0; + volatile double va = a, vb = b, vr = 0.0; double mod; int flags; feclearexcept(FE_ALL_EXCEPT); @@ -533,7 +533,7 @@ static TP_NOINLINE int tp_np_f64(int op, double a, double b, double *out) * 1.0 % 1e-310 warns overflow + invalid there; the Linux wheel (gcc) drops it and is silent. * Observed with numpy 2.5.4; mirror each. */ #if defined(__APPLE__) - vd = tp_np_divmod(va, vb, &mod); /* volatile: the quotient must really run */ + volatile double vd = tp_np_divmod(va, vb, &mod); /* volatile: the quotient must really run */ (void)vd; vr = mod; #else