KT-28102 fixes (JetBrains#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
sbogolepov · Nov 16, 2018 · 471771e · 471771e
1 parent 77be16c
commit 471771e
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 19 deletions.
diff --git a/...ubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/GlobalVariableStub.kt b/...ubGenerator/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<String> {
         val lines = mutableListOf<String>()
         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/...tor/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/MappingBridgeGeneratorImpl.kt b/...tor/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
@@ -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
@@ -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/...tubGenerator/src/main/kotlin/org/jetbrains/kotlin/native/interop/gen/jvm/StubGenerator.kt b/...tubGenerator/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<StructDecl, String>()
 
     /**
@@ -981,6 +978,7 @@ class StubGenerator(
     val libraryForCStubs = configuration.library.copy(
             includes = mutableListOf<String>().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
@@ -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
@@ -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")
diff --git 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
@@ -0,0 +1,11 @@
+---
+// KT-28065
+struct StructWithConstFields {
+    int x;
+    const int y;
+};
+
+struct StructWithConstFields getStructWithConstFields() {
+    struct StructWithConstFields result = { 111, 222 };
+    return result;
+}
diff --git a/backend.native/tests/interop/basics/globals.kt b/backend.native/tests/interop/basics/globals.kt
@@ -27,4 +27,7 @@ fun main(args: Array<String>) {
     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
@@ -38,4 +38,11 @@ fun main(args: Array<String>) {
     assertEquals(42, INT_CALL)
     assertEquals(84, CALL_SUM)
     assertEquals(5, GLOBAL_VAR)
+
+    memScoped {
+        val counter = alloc<IntVar>()
+        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
@@ -0,0 +1,10 @@
+import kotlinx.cinterop.*
+import kotlin.test.*
+import cstructs.*
+
+fun main() {
+    getStructWithConstFields().useContents {
+        assertEquals(111, x)
+        assertEquals(222, y)
+    }
+}