From 471771e8b8c7e6e63b72df16538bc034af433c56 Mon Sep 17 00:00:00 2001 From: SvyatoslavScherbina Date: Fri, 16 Nov 2018 15:14:16 +0300 Subject: [PATCH] KT-28102 fixes (#2325) * Fix support for C functions returning structs with const fields #KT-28065 Fixed * Improve support for const C globals Workaround the case when libclang improperly reports types of const variables as non-const Partially fixes KT-28102 * Improve support for C functions overloaded by macros Don't erase pointers to void* in some cases when generating C stubs Partially fixes KT-28102 --- .../native/interop/gen/GlobalVariableStub.kt | 20 +++++++++--------- .../interop/gen/MappingBridgeGeneratorImpl.kt | 5 ++++- .../kotlin/native/interop/gen/Mappings.kt | 10 ++++----- .../kotlin/native/interop/gen/TypeUtils.kt | 21 +++++++++++++++++++ .../native/interop/gen/jvm/StubGenerator.kt | 4 +--- backend.native/tests/build.gradle | 10 +++++++++ .../tests/interop/basics/cglobals.def | 5 +++++ .../tests/interop/basics/cmacros.def | 3 +++ .../tests/interop/basics/cstructs.def | 11 ++++++++++ .../tests/interop/basics/globals.kt | 3 +++ backend.native/tests/interop/basics/macros.kt | 7 +++++++ .../tests/interop/basics/structs.kt | 10 +++++++++ 12 files changed, 90 insertions(+), 19 deletions(-) create mode 100644 backend.native/tests/interop/basics/cstructs.def create mode 100644 backend.native/tests/interop/basics/structs.kt diff --git a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/GlobalVariableStub.kt b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/GlobalVariableStub.kt index ffe3553acb0..d99226c810b 100644 --- a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/GlobalVariableStub.kt +++ b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/GlobalVariableStub.kt @@ -32,6 +32,9 @@ class GlobalVariableStub(global: GlobalDecl, stubGenerator: StubGenerator) : Kot "&${global.name}" } } + + private val setterStub = object : NativeBacked {} + val header: String val getter: KotlinExpression val setter: KotlinExpression? @@ -66,14 +69,14 @@ class GlobalVariableStub(global: GlobalDecl, stubGenerator: StubGenerator) : Kot val bridgedValue = BridgeTypedKotlinValue(mirror.info.bridgedType, mirror.info.argToBridged("value")) stubGenerator.simpleBridgeGenerator.kotlinToNative( - nativeBacked = this, + nativeBacked = setterStub, returnType = BridgedType.VOID, kotlinValues = listOf(bridgedValue) ) { nativeValues -> out("${global.name} = ${mirror.info.cFromBridged( nativeValues.single(), scope, - nativeBacked = this@GlobalVariableStub + nativeBacked = setterStub )};") "" } @@ -90,8 +93,6 @@ class GlobalVariableStub(global: GlobalDecl, stubGenerator: StubGenerator) : Kot } header = buildString { - append(if (setter != null) "var" else "val") - append(" ") append(getDeclarationName(kotlinScope, global.name)) append(": ") append(kotlinType.render(kotlinScope)) @@ -105,18 +106,17 @@ class GlobalVariableStub(global: GlobalDecl, stubGenerator: StubGenerator) : Kot override fun generate(context: StubGenerationContext): Sequence { val lines = mutableListOf() if (context.nativeBridges.isSupported(this)) { - lines.add(header) + val mutable = setter != null && context.nativeBridges.isSupported(setterStub) + val kind = if (mutable) "var" else "val" + lines.add("$kind $header") lines.add(" get() = $getter") - if (setter != null) { + if (mutable) { lines.add(" set(value) { $setter }") } } else { lines.add(annotationForUnableToImport) - lines.add(header) + lines.add("val $header") lines.add(" get() = TODO()") - if (setter != null) { - lines.add(" set(value) = TODO()") - } } return lines.asSequence() diff --git a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/MappingBridgeGeneratorImpl.kt b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/MappingBridgeGeneratorImpl.kt index 28280a4c69e..2e50dc69bad 100644 --- a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/MappingBridgeGeneratorImpl.kt +++ b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/MappingBridgeGeneratorImpl.kt @@ -94,7 +94,10 @@ class MappingBridgeGeneratorImpl( "" } is RecordType -> { - out("*(${unwrappedReturnType.decl.spelling}*)${bridgeNativeValues.last()} = $nativeResult;") + val kniStructResult = "kniStructResult" + + out("${unwrappedReturnType.decl.spelling} $kniStructResult = $nativeResult;") + out("memcpy(${bridgeNativeValues.last()}, &$kniStructResult, sizeof($kniStructResult));") "" } else -> { diff --git a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/Mappings.kt b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/Mappings.kt index 2cccefc4d17..b9210dffa60 100644 --- a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/Mappings.kt +++ b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/Mappings.kt @@ -188,7 +188,7 @@ sealed class TypeInfo { } - class Pointer(val pointee: KotlinType) : TypeInfo() { + class Pointer(val pointee: KotlinType, val cPointee: Type) : TypeInfo() { override fun argToBridged(expr: String) = "$expr.rawValue" override fun argFromBridged(expr: KotlinExpression, scope: KotlinScope, nativeBacked: NativeBacked) = @@ -198,7 +198,7 @@ sealed class TypeInfo { get() = BridgedType.NATIVE_PTR override fun cFromBridged(expr: NativeExpression, scope: NativeScope, nativeBacked: NativeBacked) = - "(void*)$expr" // Note: required for JVM + "(${getPointerTypeStringRepresentation(cPointee)})$expr" override fun constructPointedType(valueType: KotlinType) = KotlinTypes.cPointerVarOf.typeWith(valueType) } @@ -423,13 +423,13 @@ fun mirror(declarationMapper: DeclarationMapper, type: Type): TypeMirror = when val pointeeType = type.pointeeType val unwrappedPointeeType = pointeeType.unwrapTypedefs() if (unwrappedPointeeType is VoidType) { - val info = TypeInfo.Pointer(KotlinTypes.cOpaque) + val info = TypeInfo.Pointer(KotlinTypes.cOpaque, pointeeType) TypeMirror.ByValue(KotlinTypes.cOpaquePointerVar, info, KotlinTypes.cOpaquePointer) } else if (unwrappedPointeeType is ArrayType) { mirror(declarationMapper, pointeeType) } else { val pointeeMirror = mirror(declarationMapper, pointeeType) - val info = TypeInfo.Pointer(pointeeMirror.pointedType) + val info = TypeInfo.Pointer(pointeeMirror.pointedType, pointeeType) TypeMirror.ByValue( KotlinTypes.cPointerVar.typeWith(pointeeMirror.pointedType), info, @@ -444,7 +444,7 @@ fun mirror(declarationMapper: DeclarationMapper, type: Type): TypeMirror = when if (type.elemType.unwrapTypedefs() is ArrayType) { elemTypeMirror } else { - val info = TypeInfo.Pointer(elemTypeMirror.pointedType) + val info = TypeInfo.Pointer(elemTypeMirror.pointedType, type.elemType) TypeMirror.ByValue( KotlinTypes.cArrayPointerVar.typeWith(elemTypeMirror.pointedType), info, diff --git a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/TypeUtils.kt b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/TypeUtils.kt index 1d97b92ce51..3f8bcf3a321 100644 --- a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/TypeUtils.kt +++ b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/TypeUtils.kt @@ -21,6 +21,9 @@ import org.jetbrains.kotlin.native.interop.indexer.* val EnumDef.isAnonymous: Boolean get() = spelling.contains("(anonymous ") // TODO: it is a hack +val StructDecl.isAnonymous: Boolean + get() = spelling.contains("(anonymous ") // TODO: it is a hack + /** * Returns the expression which could be used for this type in C code. * Note: the resulting string doesn't exactly represent this type, but it is enough for current purposes. @@ -57,6 +60,24 @@ fun Type.getStringRepresentation(): String = when (this) { else -> throw kotlin.NotImplementedError() } +fun getPointerTypeStringRepresentation(pointee: Type): String = + (getStringRepresentationOfPointee(pointee) ?: "void") + "*" + +private fun getStringRepresentationOfPointee(type: Type): String? { + val unwrapped = type.unwrapTypedefs() + + return when (unwrapped) { + is PrimitiveType -> unwrapped.getStringRepresentation() + is PointerType -> getStringRepresentationOfPointee(unwrapped.pointeeType)?.plus("*") + is RecordType -> if (unwrapped.decl.isAnonymous || unwrapped.decl.spelling == "struct __va_list_tag") { + null + } else { + unwrapped.decl.spelling + } + else -> null + } +} + private val ObjCQualifiedPointer.protocolQualifier: String get() = if (this.protocols.isEmpty()) "" else " <${protocols.joinToString { it.name }}>" diff --git a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/StubGenerator.kt b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/StubGenerator.kt index e781f30fa2b..f9809bc9dc5 100644 --- a/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/StubGenerator.kt +++ b/Interop/StubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/StubGenerator.kt @@ -74,9 +74,6 @@ class StubGenerator( typedefNames.toSet() } - val StructDecl.isAnonymous: Boolean - get() = spelling.contains("(anonymous ") // TODO: it is a hack - val anonymousStructKotlinNames = mutableMapOf() /** @@ -981,6 +978,7 @@ class StubGenerator( val libraryForCStubs = configuration.library.copy( includes = mutableListOf().apply { add("stdint.h") + add("string.h") if (platform == KotlinPlatform.JVM) { add("jni.h") } diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 663d4ae9c8d..103c3364e34 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -2775,6 +2775,10 @@ kotlinNativeInterop { defFile 'interop/basics/cunsupported.def' } + cstructs { + defFile 'interop/basics/cstructs.def' + } + if (isMac()) { objcSmoke { defFile 'interop/objc/objcSmoke.def' @@ -2868,6 +2872,12 @@ task interop_unsupported(type: RunInteropKonanTest) { interop = 'cunsupported' } +task interop_structs(type: RunInteropKonanTest) { + disabled = (project.testTarget == 'wasm32') // No interop for wasm yet. + source = "interop/basics/structs.kt" + interop = 'cstructs' +} + task interop_echo_server(type: RunInteropKonanTest) { disabled = (project.testTarget == 'wasm32') // No interop for wasm yet. if (!isMac()) { diff --git a/backend.native/tests/interop/basics/cglobals.def b/backend.native/tests/interop/basics/cglobals.def index 6472aea84e1..0dfeb14627a 100644 --- a/backend.native/tests/interop/basics/cglobals.def +++ b/backend.native/tests/interop/basics/cglobals.def @@ -26,3 +26,8 @@ MyInt g7; // Test property name mangling: struct g1 {}; struct g1_ {}; + +typedef void* voidptr; +_Pragma("clang assume_nonnull begin") +const voidptr g8 = 0x1, g9 = 0x2; +_Pragma("clang assume_nonnull end") \ No newline at end of file diff --git a/backend.native/tests/interop/basics/cmacros.def b/backend.native/tests/interop/basics/cmacros.def index 1f086802395..1097d534d1d 100644 --- a/backend.native/tests/interop/basics/cmacros.def +++ b/backend.native/tests/interop/basics/cmacros.def @@ -58,3 +58,6 @@ int global_var = 5; #define BAD1 bar #define BAD2 5; #define BAD3 { foo(); } + +void increment(int* counter); +#define increment(counter) { (*(counter))++; } diff --git a/backend.native/tests/interop/basics/cstructs.def b/backend.native/tests/interop/basics/cstructs.def new file mode 100644 index 00000000000..d085f15d611 --- /dev/null +++ b/backend.native/tests/interop/basics/cstructs.def @@ -0,0 +1,11 @@ +--- +// KT-28065 +struct StructWithConstFields { + int x; + const int y; +}; + +struct StructWithConstFields getStructWithConstFields() { + struct StructWithConstFields result = { 111, 222 }; + return result; +} \ No newline at end of file diff --git a/backend.native/tests/interop/basics/globals.kt b/backend.native/tests/interop/basics/globals.kt index e542f9dba57..9a48e87db72 100644 --- a/backend.native/tests/interop/basics/globals.kt +++ b/backend.native/tests/interop/basics/globals.kt @@ -27,4 +27,7 @@ fun main(args: Array) { assert(g5[0] == 16) assert(g6 == g3.ptr) + + assert(g8.toLong() == 0x1L) + assert(g9.toLong() == 0x2L) } diff --git a/backend.native/tests/interop/basics/macros.kt b/backend.native/tests/interop/basics/macros.kt index c28b931ee30..826f3e70887 100644 --- a/backend.native/tests/interop/basics/macros.kt +++ b/backend.native/tests/interop/basics/macros.kt @@ -38,4 +38,11 @@ fun main(args: Array) { assertEquals(42, INT_CALL) assertEquals(84, CALL_SUM) assertEquals(5, GLOBAL_VAR) + + memScoped { + val counter = alloc() + counter.value = 42 + increment(counter.ptr) + assertEquals(43, counter.value) + } } diff --git a/backend.native/tests/interop/basics/structs.kt b/backend.native/tests/interop/basics/structs.kt new file mode 100644 index 00000000000..b340c34f723 --- /dev/null +++ b/backend.native/tests/interop/basics/structs.kt @@ -0,0 +1,10 @@ +import kotlinx.cinterop.* +import kotlin.test.* +import cstructs.* + +fun main() { + getStructWithConstFields().useContents { + assertEquals(111, x) + assertEquals(222, y) + } +} \ No newline at end of file