New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SR-4827] Incorrect result resolving type for Obj-C block #47404
Comments
I was wondering when someone was going to catch us on this. Currently there's no way to get from the Objective-C class name "SomeClass" back to the Swift class "_ObjCSomeClass"—there's no connection between the two without looking at every class in the program for the one with the given name. But that's clearly the right thing to do if you really need to write code like this. |
That's basically going to be the implementation strategy if anyone wants to take this: 1. Do a regular lookup, the way we always do. I'm tagging this as a StarterBug for someone to add step 2; don't feel like you have to do step 3 as well. That will at least prevent the miscompile. |
Comment by Ben A (JIRA) @belkadan awesome thanks for the explanation and notes for someone who'd work on this! |
Comment by Leon Li (JIRA) Hey Jordan & Ben. I would really like to fix this bug. As it is my first time looking at Swift's source code, it would be really good if you can give me more detailed instructions like which file and/or function I should start looking at and what are the related classes/data structures. Cheers! |
Sure![]( I do suggest just aiming to do the Step 2 part first before moving on to Step 3. The relevant code here is in ImportDecl.cpp under {{resolveSwiftDeclImpl}}; what you'd want to do is see if the Swift declaration's attributes ( When you've got something working, you can modify one of the test cases inside test/ClangImporter/MixedSource/, and test (see docs/Testing.rst), then open a PR on the Swift repo. Feel free to tag me as a reviewer when you get there! (And of course to ask questions along the way.) |
Comment by Leon Li (JIRA) Thank you Jordan for the instructions. I was able to reproduce the project with a simple Xcode project. Now I am trying to run a specific test case to reproduce the problem. I don't really know how to do this. Ideally it would be good if I can write an unit test case in Xcode which I can run within the IDE. I looked at 'test/ClangImporter/MixedSource/' and not sure which test I should run. And how I can run those tests within Xcode? Many thanks! |
Comment by Benjamin Encz (JIRA) FYI, this issue is still occurring in Swift 3.2. |
Resetting assignee for all Starter Bugs not modified since 2018. |
I've submitted a pull request for this. It pretty much follows the implementation steps that Jordan explained. This particular bug could be resolved by just excluding Swift declarations that weren't being exposed to Obj-C. |
We had to revert this for now because we found a case where people were relying on the old behavior ( |
Back to Resolved now that #27921 is in. |
Environment
macOS 10.12.4
Swift 3.1
Xcode 8.3.2
Also:
macOS 10.12.5
Swift 3.2
Xcode 9.0 beta (9M136h)
Additional Detail from JIRA
md5: 5590c8a0ad8e21dbbb40a05d1c250d51
is duplicated by:
Issue Description:
Consider the following files:
SampleClass.h
SampleClass.m
test.swift
This can be run using the following commands:
The program runs and prints the following:
The type being printed here is incorrect. This should be printing
Optional<_ObjCSomeFactory>
. The bad type-checker result can be further exposed by annotating the block like so with more type information:Updating the code like this causes a compiler error:
By adding these type annotations, it's exposed that the block in SampleClass.h has been resolved to have a parameter of type
SomeFactory?
, when it should be_ObjCSomeFactory?
.As for the segfault (in the actual program that I derived this example from) Instruments showed that it was crashing because the closure executed by
SampleClass
inSomethingElse.foo
ends up over-releasingfactory
when the closure is cleaned up (in_dispatch_call_block_and_release
). This seems like it might be related to the bad type resolution?We stared at this code for a long time trying to figure out where there could possible be a memory management bug here, and then we arrived at the fact that the types, as Swift understands them, in the closure that's implemented in Swift, but defined and called in Obj-C, are incorrect. Those types being understood as correct seems like an important prerequisite for Swift getting the memory management correct in this scenario.
The text was updated successfully, but these errors were encountered: