Codex/fix swift bridge linkage - #454
Conversation
Codex/sync upstream 0.9.8
|
@SpectralDragon is attempting to deploy a commit to the sqlitecloud Team on Vercel. A member of the Team first needs to authorize it. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 794ec424ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // register objc.loadClass method | ||
| gravity_gc_setenabled(vm, false); | ||
|
|
||
| gravity_class_t *c = objc_class_load(vm, "NSAlert"); |
There was a problem hiding this comment.
Registering NSAlert directly removes the global objc bridge and its loadClass/exec methods, so existing Gravity programs using objc.loadClass(...) can no longer dynamically expose any Objective-C class. It also makes objc_register fail outright on platforms where NSAlert is unavailable; restore the generic bridge registration rather than hard-coding this AppKit class.
Useful? React with 👍 / 👎.
| // gravity_instance_setxdata(instance, (__bridge void *)(obj2)); | ||
| // } | ||
| // RETURN_NOVALUE(); | ||
| return false; |
There was a problem hiding this comment.
When a bridged Objective-C initializer has arguments (nargs > 1), this unconditional failure is now the only reachable path because the previous initializer invocation was commented out. Consequently, constructions such as SomeClass(value) always fail even when the selector and arguments are valid; retain the existing zero-argument handling while restoring the argument-bearing initialization path.
Useful? React with 👍 / 👎.
| if arguments.count > method.argsCount { | ||
| return GravityReturn.error("Passed more arguments, then expected. Method \(method.name) expected \(method.argsCount) arguments, but passed \(arguments.count) arguments.", vm: vm) |
There was a problem hiding this comment.
If Gravity calls an exported Swift method with fewer arguments than declared, this check succeeds and the generated MethodDescriptor wrapper immediately indexes missing elements such as args[0], trapping the host process instead of reporting a script error. Validate that arguments.count equals method.argsCount (the same issue also exists in the constructor path) before invoking the wrapper.
Useful? React with 👍 / 👎.
| return delegate.virtalMachine(vm, didRequestCloneFor: value).xData | ||
| } | ||
|
|
||
| return objptr |
There was a problem hiding this comment.
For a bridged instance cloned through Gravity's clone() operation, returning the original objptr assigns the same retained _ValueBox pointer to both instances without adding an ownership retain. Each instance later invokes bridgeFree, so the first destruction releases the box and the second accesses/releases a dangling pointer; retain the shared box for the clone or create an independently owned clone.
Useful? React with 👍 / 👎.
| rhsPtr: UnsafeMutableRawPointer? | ||
| ) -> Bool { | ||
| guard let vm = GravityVirtualMachine.getVM(vmPointer) else { fatalError("Cannot found Virtual Machine") } | ||
| return vm.delegate.virtualMachineBridgeEquals(vm, lhsValue: GSValue(object: lhsPtr, in: vm), rhsValue: GSValue(object: lhsPtr, in: vm)) |
There was a problem hiding this comment.
Whenever two distinct bridged instances reach the delegate equality fallback, both callback arguments are constructed from lhsPtr, so the delegate never sees rhsPtr and comparisons can report equality for unrelated objects. Construct rhsValue from rhsPtr instead.
Useful? React with 👍 / 👎.
| sourcePtr, | ||
| source.count, | ||
| 0, // fileId |
There was a problem hiding this comment.
For source containing any multibyte Unicode character, String.count reports grapheme count while gravity_compiler_run expects the byte length of the UTF-8 buffer supplied by withCString; the compiler therefore receives a truncated source buffer and may produce syntax errors or omit trailing code. Pass source.utf8.count here, and likewise use UTF-8 byte counts in the other new length-taking C calls.
Useful? React with 👍 / 👎.
1 participant
No description provided.