Skip to content

Commit

Permalink
I did some dubious work in the 4.1 migration, that is, I was using
Browse files Browse the repository at this point in the history
withUnsafePointer(to:) and returning the address that was calculated,
because I figured "I know this can not go away".

Well, it turns out, that I did not know this very well to begin with,
and when running the code in Optimize mode in some scenarios, the
value did go away - just like the Swift documentation had warned.

Let this be another lesson that hubris never pays off.

What I was doing was generating something like this to pass a pointer to the handle of a parameter:

func demo (parameter: SomeWrapped, another: SomeWrapped) {
   var args: [...] = [
       withUnsafePointer (to: &parameter.handle) { p in return p }
       withUnsafePointer (to: &another.handle) { p in return p }
   ]
   callGodotWith (args)
}

The value of `p` in the callback invoked by withUnsafePointer is only
valid for the duration of the block, but like I said, I though I could
get away with it.

Now instead of generating code like this, I nest the calls to withUnsafePointer, like this:

func demo (parameter: SomeWrapped, another: SomeWrapped) {
    var args: [] = []
    withUnsafePointer (to: parameter.handle) { p1 in
        args.append (p1)
        withUnsafePointer (to: another.handle) { p2 in
	    args.append (p2)
	    callGodotWith (args)
	}
    }
}
  • Loading branch information
migueldeicaza committed Sep 19, 2023
1 parent 70ed829 commit 4394cf4
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 29 deletions.
4 changes: 2 additions & 2 deletions Generator/Generator/DocModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import XMLCoder
struct DocClass: Codable {
@Attribute var name: String
@Attribute var inherits: String?
@Attribute var version: String
@Attribute var version: String?
var brief_description: String
var description: String
var tutorials: [DocTutorial]
Expand All @@ -24,7 +24,7 @@ struct DocClass: Codable {

struct DocBuiltinClass: Codable {
@Attribute var name: String
@Attribute var version: String
@Attribute var version: String?
var brief_description: String
var description: String
var tutorials: [DocTutorial]
Expand Down
59 changes: 35 additions & 24 deletions Generator/Generator/MethodGen.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ func isRefParameterOptional (className: String, method: String, arg: String) ->
}
}


/// Generates a method definition
/// - Parameters:
/// - p: Our printer to generate the method
Expand All @@ -88,7 +87,6 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
var finalp: String
// Default method name
var methodName: String = godotMethodToSwift (method.name)

let instanceOrStatic = method.isStatic ? " static" : ""
var inline = ""
if let methodHash = method.hash {
Expand Down Expand Up @@ -143,9 +141,15 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:

varArgSetup += "for idx in 0..<arguments.count {\n"
varArgSetup += " content [idx] = arguments [idx].content\n"
varArgSetup += " args.append (content.baseAddress! + idx)\n"
varArgSetup += " _args.append (content.baseAddress! + idx)\n"
varArgSetup += "}\n"
}
let godotReturnType = method.returnValue?.type
let godotReturnTypeIsReferenceType = classMap [godotReturnType ?? ""] != nil
let returnOptional = godotReturnTypeIsReferenceType && isReturnOptional(className: className, method: method.name)
let returnType = getGodotType (method.returnValue) + (returnOptional ? "?" : "")

var withUnsafeCallNestLevel = 0

if let margs = method.arguments {
for arg in margs {
Expand Down Expand Up @@ -177,7 +181,7 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
if args != "" { args += ", "}
args += "_ arguments: Variant..."
}
argSetup += "var args: [UnsafeRawPointer?] = [\n"
argSetup += "var _args: [UnsafeRawPointer?] = []\n"
for arg in margs {
// When we move from GString to String in the public API
// if arg.type == "String" {
Expand Down Expand Up @@ -216,37 +220,31 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
}
}
}
argSetup += " "
if refParameterIsOptional {
argSetup += "\(escapeSwift (argref)) == nil ? nil : UnsafeRawPointer (withUnsafePointer(to: \(escapeSwift (argref))!.handle) { p in p }),"
//argSetup += "UnsafeRawPointer(\(needAddress)\(escapeSwift(argref))?.handle ?? nil),"
// With Godot 4.1 we need to pass the address of the handle
if refParameterIsOptional || optstorage == ".handle" {
let prefix = String(repeating: " ", count: withUnsafeCallNestLevel * 4)
let ea = escapeSwift(argref)
let retFromWith = returnType != "" ? "return " : ""
let deref = refParameterIsOptional ? "?" : ""
argSetup += "\(prefix)\(retFromWith)withUnsafePointer (to: \(ea)\(deref).handle) { p\(withUnsafeCallNestLevel) in\n\(prefix)_args.append (\(ea) == nil ? nil : p\(withUnsafeCallNestLevel))\n"
withUnsafeCallNestLevel += 1
} else {
// With Godot 4.1 we need to pass the address of the handle
if optstorage == ".handle" {
argSetup += "UnsafeRawPointer (withUnsafePointer(to: \(escapeSwift (argref)).handle) { p in p }),"
} else {
argSetup += "UnsafeRawPointer(\(needAddress)\(escapeSwift(argref))\(optstorage)),"
}
let prefix = String(repeating: " ", count: withUnsafeCallNestLevel * 4)
argSetup += "\(prefix)_args.append (UnsafeRawPointer(\(needAddress)\(escapeSwift(argref))\(optstorage)))\n"
}
}
argSetup += "]"
argSetup += varArgSetupInit
argSetup += varArgSetup
} else if method.isVararg {
// No regular arguments, check if these are varargs
if method.isVararg {
args = "_ arguments: Variant..."
}
argSetup += "var args: [UnsafeRawPointer?] = []\n"
argSetup += "var _args: [UnsafeRawPointer?] = []\n"
argSetup += varArgSetupInit
argSetup += varArgSetup
}

let godotReturnType = method.returnValue?.type
let godotReturnTypeIsReferenceType = classMap [godotReturnType ?? ""] != nil
let returnOptional = godotReturnTypeIsReferenceType && isReturnOptional(className: className, method: method.name)
let returnType = getGodotType (method.returnValue) + (returnOptional ? "?" : "")

if inline != "" {
p (inline)
}
Expand All @@ -262,6 +260,8 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
p ("@discardableResult")
}
p ("\(visibility)\(instanceOrStatic) \(finalp)func \(methodName) (\(args))\(returnType != "" ? "-> " + returnType : "")") {
// We will change the nest level in the body after we print out the prefix of the nested withUnsafe calls

if method.hash == nil {
if let godotReturnType {
p (makeDefaultReturn (godotType: godotReturnType))
Expand Down Expand Up @@ -293,7 +293,11 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
if argSetup != "" {
p (argSetup)
}
let ptrArgs = (args != "") ? "&args" : "nil"
if withUnsafeCallNestLevel > 0 {
p.indent += withUnsafeCallNestLevel
}

let ptrArgs = (args != "") ? "&_args" : "nil"
let ptrResult: String
if returnType != "" {
if method.isVararg {
Expand Down Expand Up @@ -321,13 +325,13 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
case .class:
let instanceHandle = method.isStatic ? "nil, " : "UnsafeMutableRawPointer (mutating: handle), "
if method.isVararg {
p ("gi.object_method_bind_call (\(className).method_\(method.name), \(instanceHandle)\(ptrArgs), Int64 (args.count), \(ptrResult), nil)")
p ("gi.object_method_bind_call (\(className).method_\(method.name), \(instanceHandle)\(ptrArgs), Int64 (_args.count), \(ptrResult), nil)")
} else {
p ("gi.object_method_bind_ptrcall (\(className).method_\(method.name), \(instanceHandle)\(ptrArgs), \(ptrResult))")
}
case .utility:
if method.isVararg {
p ("\(bindName) (\(ptrResult), \(ptrArgs), Int32 (args.count))")
p ("\(bindName) (\(ptrResult), \(ptrArgs), Int32 (_args.count))")
} else {
p ("\(bindName) (\(ptrResult), \(ptrArgs), Int32 (\(method.arguments?.count ?? 0)))")
}
Expand Down Expand Up @@ -366,6 +370,13 @@ func methodGen (_ p: Printer, method: MethodDefinition, className: String, cdef:
p ("return _result")
}
}

// Unwrap the nested calls to 'withUnsafePointer'
while withUnsafeCallNestLevel > 0 {
withUnsafeCallNestLevel -= 1
p.indent -= 1
p ("}")
}
}
}
return registerVirtualMethodName
Expand Down
4 changes: 4 additions & 0 deletions Generator/Generator/Printer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ public class Printer {
func b (_ str: String, suffix: String = "", block: () -> ()) {
p (str + " {")
indent += 1
let saved = indent
block ()
if indent != saved {
print ("Indentation out of sync, the nested block messed with indentation")
}
indent -= 1
p ("}\(suffix)\n")
}
Expand Down
8 changes: 5 additions & 3 deletions Generator/Generator/main.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@ var docRoot = args.count > 3 ? args [3] : "/Users/miguel/cvs/godot-master/doc"
let outputDir = args.count > 2 ? args [2] : generatorOutput

if args.count < 2 {
print ("Usage is: generator [godot-main-directory [output-directory]]")
print ("where godot-main-directory contains api.json and builtin-api.json")
print ("If unspecified, this will default to the built-in versions")
print ("Usage is: generator path-to-extension-api output-directory doc-directory")
print ("- path-to-extensiona-ppi is the full path to extension_api.json from Godot")
print ("- output-directory is where the files will be placed")
print ("- doc-directory is the Godot documentation resides (godot/doc)")
print ("Running with Miguel's testing defaults")
}

let jsonData = try! Data(contentsOf: URL(fileURLWithPath: jsonFile))
Expand Down

0 comments on commit 4394cf4

Please sign in to comment.