mirror of
https://github.com/nim-lang/Nim.git
synced 2026-01-16 17:47:11 +00:00
Objects containing `importc` fields without `completeStruct` fail to compile when used as const/static. The C codegen generates "aggregate initialization" which is invalid for opaque types. Fixes #25405. Nim code: ```nim type OpaqueInt {.importc: "_Atomic int", nodecl.} = object ContainsImportc = object normal: int opaque: OpaqueInt const c = default(ContainsImportc) ``` Resulting C code: ```c // Invalid C - cannot aggregate-init opaque type NIM_CONST ContainsImportc c = {((NI) 0), {}}; ^^ error: illegal initializer type ``` ## Solution Fix in `ccgexprs.nim`: 1. Skip opaque importc fields when building aggregate initializers 2. Use "designated initializers" (`siNamedStruct`) when opaque fields are present to avoid positional misalignment ```c // Valid C: // - opaque field is omitted and implicitly zero-initialized by C // - other fields are explitly named and initialized NIM_CONST ContainsImportc c = {.normal = ((NI) 0)}; ``` This correctly handles the case where the opaque fields might be in any order. A field is considered "opaque importc" if: - Has `sfImportc` flag - Does NOT have `tfCompleteStruct` flag - Either has `tfIncompleteStruct` OR is an object with no visible fields The `containsOpaqueImportcField` proc recursively checks all object fields, including nested objects and variant branches. Anonymous unions (from variant objects) are handled by passing an empty field name, which skips the `.fieldname = ` prefix since C anonymous unions have no field name. Note that initialization for structs without opaque importc fields remains the same as before this changeset. ## Test Coverage `tests/ccgbugs/timportc_field_init.nim` covers: - Simple struct with one importc field - Nested struct containing struct with importc field - Variant object (case object) with importc field in a branch - Array of structs with importc fields - Tuple containing struct with importc field - `completeStruct` importc types (still use aggregate init) - Sandwich case (opaque field between two non-opaque fields) - Fields with different C names (`{.importc: "c_name".}`, `{.exportc.}`) - `{.packed.}` structs with opaque fields - `{.union.}` types with opaque fields - Deep nesting (3+ levels) - Multiple opaque fields with renamed fields between them
This commit is contained in:
elijahr
@@ -154,14 +154,14 @@ template addField(builder: var Builder, constr: var StructInitializer, name: str
|
||||
# no name, can just add value
|
||||
valueBody
|
||||
of siOrderedStruct:
|
||||
# no name, can just add value on C
|
||||
assert name.len != 0, "name has to be given for struct initializer field"
|
||||
# positional init - name not used in output (empty allowed for anonymous unions)
|
||||
valueBody
|
||||
of siNamedStruct:
|
||||
assert name.len != 0, "name has to be given for struct initializer field"
|
||||
builder.add(".")
|
||||
builder.add(name)
|
||||
builder.add(" = ")
|
||||
# designated init - empty name for anonymous unions (skips .name = prefix)
|
||||
if name.len != 0:
|
||||
builder.add(".")
|
||||
builder.add(name)
|
||||
builder.add(" = ")
|
||||
valueBody
|
||||
|
||||
proc finishStructInitializer(builder: var Builder, constr: StructInitializer) =
|
||||
|
||||
@@ -3713,6 +3713,64 @@ proc expr(p: BProc, n: PNode, d: var TLoc) =
|
||||
of nkMixinStmt, nkBindStmt, nkReplayAction: discard
|
||||
else: internalError(p.config, n.info, "expr(" & $n.kind & "); unknown node kind")
|
||||
|
||||
proc isOpaqueImportcType(t: PType): bool =
|
||||
# importc type without completeStruct that can't use aggregate init (e.g. C11 _Atomic)
|
||||
if t.sym != nil and sfImportc in t.sym.flags:
|
||||
if tfCompleteStruct notin t.flags:
|
||||
if tfIncompleteStruct in t.flags:
|
||||
return true
|
||||
if t.kind == tyObject and (t.n == nil or t.n.len == 0):
|
||||
return true
|
||||
return false
|
||||
|
||||
proc containsOpaqueImportcField(typ: PType): bool
|
||||
|
||||
proc containsOpaqueImportcFieldAux(t: PType; n: PNode): bool =
|
||||
if n == nil: return false
|
||||
case n.kind
|
||||
of nkRecList:
|
||||
for child in n.sons:
|
||||
if containsOpaqueImportcFieldAux(t, child):
|
||||
return true
|
||||
of nkRecCase:
|
||||
if containsOpaqueImportcFieldAux(t, n[0]):
|
||||
return true
|
||||
for i in 1..<n.len:
|
||||
let branch = n[i]
|
||||
if branch.kind == nkOfBranch or branch.kind == nkElse:
|
||||
if containsOpaqueImportcFieldAux(t, branch.lastSon):
|
||||
return true
|
||||
of nkSym:
|
||||
if containsOpaqueImportcField(n.sym.typ):
|
||||
return true
|
||||
else:
|
||||
discard
|
||||
return false
|
||||
|
||||
proc containsOpaqueImportcField(typ: PType): bool =
|
||||
# Check if type contains opaque importc fields that need designated initializers
|
||||
if typ == nil: return false
|
||||
let t = skipTypes(typ, abstractRange+{tyOwned}-{tyTypeDesc})
|
||||
if isOpaqueImportcType(t):
|
||||
return true
|
||||
case t.kind
|
||||
of tyObject:
|
||||
if t.baseClass != nil:
|
||||
if containsOpaqueImportcField(t.baseClass):
|
||||
return true
|
||||
if containsOpaqueImportcFieldAux(t, t.n):
|
||||
return true
|
||||
of tyTuple:
|
||||
for i, a in t.ikids:
|
||||
if containsOpaqueImportcField(a):
|
||||
return true
|
||||
of tyArray:
|
||||
if containsOpaqueImportcField(t.elementType):
|
||||
return true
|
||||
else:
|
||||
discard
|
||||
return false
|
||||
|
||||
proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo; result: var Builder) =
|
||||
var t = skipTypes(typ, abstractRange+{tyOwned}-{tyTypeDesc})
|
||||
case t.kind
|
||||
@@ -3743,24 +3801,34 @@ proc getDefaultValue(p: BProc; typ: PType; info: TLineInfo; result: var Builder)
|
||||
result.addField(closureInit, name = "ClE_0"):
|
||||
result.add(NimNil)
|
||||
of tyObject:
|
||||
# Use designated initializers when opaque importc fields present
|
||||
var objInit: StructInitializer
|
||||
result.addStructInitializer(objInit, kind = siOrderedStruct):
|
||||
let initKind = if containsOpaqueImportcField(t): siNamedStruct else: siOrderedStruct
|
||||
result.addStructInitializer(objInit, kind = initKind):
|
||||
getNullValueAuxT(p, t, t, t.n, nil, result, objInit, true, info)
|
||||
of tyTuple:
|
||||
# Use designated initializers when opaque importc fields present
|
||||
var tupleInit: StructInitializer
|
||||
result.addStructInitializer(tupleInit, kind = siOrderedStruct):
|
||||
let initKind = if containsOpaqueImportcField(t): siNamedStruct else: siOrderedStruct
|
||||
result.addStructInitializer(tupleInit, kind = initKind):
|
||||
if p.vccAndC and t.isEmptyTupleType:
|
||||
result.addField(tupleInit, name = "dummy"):
|
||||
result.addIntValue(0)
|
||||
for i, a in t.ikids:
|
||||
result.addField(tupleInit, name = "Field" & $i):
|
||||
getDefaultValue(p, a, info, result)
|
||||
let elemTyp = skipTypes(a, abstractRange+{tyOwned}-{tyTypeDesc})
|
||||
if not isOpaqueImportcType(elemTyp):
|
||||
result.addField(tupleInit, name = "Field" & $i):
|
||||
getDefaultValue(p, a, info, result)
|
||||
of tyArray:
|
||||
var arrInit: StructInitializer
|
||||
result.addStructInitializer(arrInit, kind = siArray):
|
||||
for i in 0..<toInt(lengthOrd(p.config, t.indexType)):
|
||||
result.addField(arrInit, name = ""):
|
||||
getDefaultValue(p, t.elementType, info, result)
|
||||
let elemTyp = skipTypes(t.elementType, abstractRange+{tyOwned}-{tyTypeDesc})
|
||||
if isOpaqueImportcType(elemTyp):
|
||||
result.add "{0}"
|
||||
else:
|
||||
var arrInit: StructInitializer
|
||||
result.addStructInitializer(arrInit, kind = siArray):
|
||||
for i in 0..<toInt(lengthOrd(p.config, t.indexType)):
|
||||
result.addField(arrInit, name = ""):
|
||||
getDefaultValue(p, t.elementType, info, result)
|
||||
#result = rope"{}"
|
||||
of tyOpenArray, tyVarargs:
|
||||
var openArrInit: StructInitializer
|
||||
@@ -3815,8 +3883,7 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode,
|
||||
var fieldName: string = ""
|
||||
if b.kind == nkRecList and not isEmptyCaseObjectBranch(b):
|
||||
fieldName = "_" & mangleRecFieldName(p.module, obj[0].sym) & "_" & $selectedBranch
|
||||
result.addField(init, name = "<anonymous union>"):
|
||||
# XXX figure out name for the union, see use of `addAnonUnion`
|
||||
result.addField(init, name = ""): # anonymous union
|
||||
var branchInit: StructInitializer
|
||||
result.addStructInitializer(branchInit, kind = siNamedStruct):
|
||||
result.addField(branchInit, name = fieldName):
|
||||
@@ -3825,8 +3892,7 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode,
|
||||
getNullValueAux(p, t, b, constOrNil, result, branchObjInit, isConst, info)
|
||||
elif b.kind == nkSym:
|
||||
fieldName = mangleRecFieldName(p.module, b.sym)
|
||||
result.addField(init, name = "<anonymous union>"):
|
||||
# XXX figure out name for the union, see use of `addAnonUnion`
|
||||
result.addField(init, name = ""): # anonymous union
|
||||
var branchInit: StructInitializer
|
||||
result.addStructInitializer(branchInit, kind = siNamedStruct):
|
||||
result.addField(branchInit, name = fieldName):
|
||||
@@ -3841,6 +3907,9 @@ proc getNullValueAux(p: BProc; t: PType; obj, constOrNil: PNode,
|
||||
|
||||
of nkSym:
|
||||
let field = obj.sym
|
||||
let fieldTyp = skipTypes(field.typ, abstractRange+{tyOwned}-{tyTypeDesc})
|
||||
if isOpaqueImportcType(fieldTyp):
|
||||
return # C zero-initializes omitted fields
|
||||
let sname = mangleRecFieldName(p.module, field)
|
||||
result.addField(init, name = sname):
|
||||
block fieldInit:
|
||||
@@ -3885,11 +3954,10 @@ proc getNullValueAuxT(p: BProc; orig, t: PType; obj, constOrNil: PNode,
|
||||
|
||||
proc genConstObjConstr(p: BProc; n: PNode; isConst: bool; result: var Builder) =
|
||||
let t = n.typ.skipTypes(abstractInstOwned)
|
||||
#if not isObjLackingTypeField(t) and not p.module.compileToCpp:
|
||||
# result.addf("{$1}", [genTypeInfo(p.module, t)])
|
||||
# inc count
|
||||
# Use designated initializers when opaque importc fields present
|
||||
var objInit: StructInitializer
|
||||
result.addStructInitializer(objInit, kind = siOrderedStruct):
|
||||
let initKind = if t.kind == tyObject and containsOpaqueImportcField(t): siNamedStruct else: siOrderedStruct
|
||||
result.addStructInitializer(objInit, kind = initKind):
|
||||
if t.kind == tyObject:
|
||||
getNullValueAuxT(p, t, t, t.n, n, result, objInit, isConst, n.info)
|
||||
|
||||
|
||||
178
tests/ccgbugs/timportc_field_init.nim
Normal file
178
tests/ccgbugs/timportc_field_init.nim
Normal file
@@ -0,0 +1,178 @@
|
||||
discard """
|
||||
targets: "c cpp"
|
||||
"""
|
||||
# Test const initialization of objects with opaque importc fields (e.g. FILE from stdio.h)
|
||||
|
||||
type OpaqueFile {.importc: "FILE", header: "<stdio.h>".} = object
|
||||
|
||||
type
|
||||
SimpleStruct = object
|
||||
normal: int
|
||||
opaque: OpaqueFile
|
||||
|
||||
NestedStruct = object
|
||||
inner: SimpleStruct
|
||||
value: float
|
||||
|
||||
VariantStruct = object
|
||||
case kind: bool
|
||||
of true:
|
||||
opaque: OpaqueFile
|
||||
of false:
|
||||
normal: int
|
||||
|
||||
ArrayElementStruct = object
|
||||
id: int
|
||||
atom: OpaqueFile
|
||||
|
||||
const simple = default(SimpleStruct)
|
||||
const nested = default(NestedStruct)
|
||||
const variant = default(VariantStruct)
|
||||
const arr = default(array[3, ArrayElementStruct])
|
||||
|
||||
static:
|
||||
doAssert simple.normal == 0
|
||||
doAssert nested.value == 0.0
|
||||
doAssert arr[0].id == 0
|
||||
|
||||
# completeStruct types use normal aggregate init
|
||||
type CompleteImportc {.importc: "int", completeStruct, nodecl.} = object
|
||||
value: cint
|
||||
|
||||
type StructWithComplete = object
|
||||
c: CompleteImportc
|
||||
x: int
|
||||
|
||||
const withComplete = default(StructWithComplete)
|
||||
|
||||
type TupleWithOpaque = tuple[x: int, s: SimpleStruct, y: float]
|
||||
const tupleVal = default(TupleWithOpaque)
|
||||
|
||||
# Sandwich: opaque between non-opaque fields requires designated init
|
||||
type SandwichStruct = object
|
||||
first: int
|
||||
opaque: OpaqueFile
|
||||
last: float
|
||||
|
||||
const sandwich = default(SandwichStruct)
|
||||
|
||||
static:
|
||||
doAssert withComplete.x == 0
|
||||
doAssert tupleVal.x == 0
|
||||
doAssert sandwich.first == 0
|
||||
doAssert sandwich.last == 0.0
|
||||
|
||||
proc useSimple(s: ptr SimpleStruct) {.exportc, noinline.} = discard
|
||||
proc useNested(s: ptr NestedStruct) {.exportc, noinline.} = discard
|
||||
proc useArr(a: ptr array[3, ArrayElementStruct]) {.exportc, noinline.} = discard
|
||||
proc useComplete(s: ptr StructWithComplete) {.exportc, noinline.} = discard
|
||||
proc useVariant(v: ptr VariantStruct) {.exportc, noinline.} = discard
|
||||
proc useTuple(t: TupleWithOpaque) {.exportc, noinline.} = discard
|
||||
proc useSandwich(s: ptr SandwichStruct) {.exportc, noinline.} = discard
|
||||
|
||||
useSimple(simple.addr)
|
||||
useNested(nested.addr)
|
||||
useArr(arr.addr)
|
||||
useComplete(withComplete.addr)
|
||||
useVariant(variant.addr)
|
||||
useTuple(tupleVal)
|
||||
useSandwich(sandwich.addr)
|
||||
|
||||
# Edge cases: different C/Nim names
|
||||
type OpaqueWithCName {.importc: "FILE", header: "<stdio.h>".} = object
|
||||
|
||||
type StructWithRenamedField = object
|
||||
nimName {.importc: "c_name".}: int
|
||||
opaque: OpaqueWithCName
|
||||
|
||||
const renamedField = default(StructWithRenamedField)
|
||||
proc useRenamedField(s: ptr StructWithRenamedField) {.exportc, noinline.} = discard
|
||||
useRenamedField(renamedField.addr)
|
||||
static: doAssert renamedField.nimName == 0
|
||||
|
||||
type NimTypeName {.importc: "int", completeStruct, nodecl.} = distinct cint
|
||||
|
||||
type StructContainingRenamedType = object
|
||||
inner: NimTypeName
|
||||
opaque: OpaqueFile
|
||||
|
||||
const withRenamedType = default(StructContainingRenamedType)
|
||||
proc useRenamedType(s: ptr StructContainingRenamedType) {.exportc, noinline.} = discard
|
||||
useRenamedType(withRenamedType.addr)
|
||||
|
||||
type StructWithExportedField = object
|
||||
nimField {.exportc: "exported_field".}: int
|
||||
opaque: OpaqueFile
|
||||
|
||||
const withExported = default(StructWithExportedField)
|
||||
proc useExportedField(s: ptr StructWithExportedField) {.exportc, noinline.} = discard
|
||||
useExportedField(withExported.addr)
|
||||
static: doAssert withExported.nimField == 0
|
||||
|
||||
type ByCopyStruct {.bycopy.} = object
|
||||
data: int
|
||||
opaque: OpaqueFile
|
||||
|
||||
const byCopyVal = default(ByCopyStruct)
|
||||
proc useByCopy(s: ByCopyStruct) {.exportc, noinline.} = discard
|
||||
useByCopy(byCopyVal)
|
||||
static: doAssert byCopyVal.data == 0
|
||||
|
||||
type PackedStruct {.packed.} = object
|
||||
a: int8
|
||||
opaque: OpaqueFile
|
||||
b: int8
|
||||
|
||||
const packedVal = default(PackedStruct)
|
||||
proc usePacked(s: ptr PackedStruct) {.exportc, noinline.} = discard
|
||||
usePacked(packedVal.addr)
|
||||
static:
|
||||
doAssert packedVal.a == 0
|
||||
doAssert packedVal.b == 0
|
||||
|
||||
type UnionWithOpaque {.union.} = object
|
||||
intVal: int
|
||||
opaque: OpaqueFile
|
||||
|
||||
const unionVal = default(UnionWithOpaque)
|
||||
proc useUnion(u: ptr UnionWithOpaque) {.exportc, noinline.} = discard
|
||||
useUnion(unionVal.addr)
|
||||
|
||||
# Deep nesting
|
||||
type DeepLevel1 = object
|
||||
field1: int
|
||||
opaque: OpaqueFile
|
||||
|
||||
type DeepLevel2 = object
|
||||
nested: DeepLevel1
|
||||
field2: float
|
||||
|
||||
type DeepLevel3 = object
|
||||
deep: DeepLevel2
|
||||
field3: int
|
||||
opaque2: OpaqueWithCName
|
||||
|
||||
const deepVal = default(DeepLevel3)
|
||||
proc useDeep(d: ptr DeepLevel3) {.exportc, noinline.} = discard
|
||||
useDeep(deepVal.addr)
|
||||
static:
|
||||
doAssert deepVal.deep.nested.field1 == 0
|
||||
doAssert deepVal.deep.field2 == 0.0
|
||||
doAssert deepVal.field3 == 0
|
||||
|
||||
# Multiple opaque fields with renamed non-opaque fields
|
||||
type MultiOpaque = object
|
||||
first {.importc: "first_field".}: int
|
||||
opaque1: OpaqueFile
|
||||
second {.importc: "second_field".}: float
|
||||
opaque2: OpaqueWithCName
|
||||
third: int
|
||||
|
||||
const multiOpaque = default(MultiOpaque)
|
||||
proc useMultiOpaque(m: ptr MultiOpaque) {.exportc, noinline.} = discard
|
||||
useMultiOpaque(multiOpaque.addr)
|
||||
|
||||
static:
|
||||
doAssert multiOpaque.first == 0
|
||||
doAssert multiOpaque.second == 0.0
|
||||
doAssert multiOpaque.third == 0
|
||||
Reference in New Issue
Block a user