-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[ClangImporter] Swift Implemented ObjC Forward Declarations #62983
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4298,19 +4298,21 @@ namespace { | |
} | ||
} | ||
|
||
if (!found) { | ||
// Go back to the first list and find classes with matching Swift names | ||
// *even if the ObjC name doesn't match.* | ||
// This shouldn't be allowed but we need it for source compatibility; | ||
// people used `@class SwiftNameOfClass` as a workaround for not | ||
// having the previous loop, and it "worked". | ||
for (auto result : swiftDeclsByName) { | ||
if (auto singleResult = dyn_cast<T>(result)) { | ||
if (isMatch(singleResult, /*baseNameMatches=*/true, | ||
/*allowObjCMismatch=*/true)) { | ||
if (found) | ||
return nullptr; | ||
found = singleResult; | ||
if (!languageVersion.isVersionAtLeast(6)) { | ||
if (!found) { | ||
// Go back to the first list and find classes with matching Swift | ||
// names *even if the ObjC name doesn't match.* This shouldn't be | ||
// allowed but we need it for source compatibility; people used | ||
// `@class SwiftNameOfClass` as a workaround for not having the | ||
// previous loop, and it "worked". | ||
for (auto result : swiftDeclsByName) { | ||
if (auto singleResult = dyn_cast<T>(result)) { | ||
if (isMatch(singleResult, /*baseNameMatches=*/true, | ||
/*allowObjCMismatch=*/true)) { | ||
if (found) | ||
return nullptr; | ||
found = singleResult; | ||
} | ||
} | ||
} | ||
} | ||
|
@@ -4338,6 +4340,35 @@ namespace { | |
return result; | ||
} | ||
} | ||
if (auto importingContext = Impl.getImportingContext()) { | ||
llvm::SmallVector<ValueDecl *> results; | ||
llvm::SmallVector<ImportedModule> importedModules; | ||
|
||
const ModuleDecl *importingSwiftModule = | ||
importingContext.getValue()->getParentModule(); | ||
|
||
if (importingSwiftModule->isNonSwiftModule()) | ||
return nullptr; | ||
|
||
const SourceFile *importingSwiftSourceFile = | ||
importingContext.getValue()->getParentSourceFile(); | ||
|
||
// TODO: Make sure we aren't bailing early in legitimate cases | ||
if (!importingSwiftSourceFile) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems there are cases where the importing DeclContext does not lie within a source file. I would've thought when the main module is already serialized and being inspected by swift-ide-test would be such a case, but it doesn't seem so. As the comment says, lets make sure by bailing here we aren't leaving something important on the table. |
||
return nullptr; | ||
|
||
importingSwiftSourceFile->getImportedModules( | ||
importedModules, ModuleDecl::ImportFilterKind::Default); | ||
|
||
for (auto &import : importedModules) { | ||
if (import.importedModule->isNonSwiftModule()) | ||
continue; | ||
|
||
if (T *result = resolveSwiftDeclImpl<T>(decl, name, hasKnownSwiftName, | ||
import.importedModule)) | ||
return result; | ||
} | ||
} | ||
return nullptr; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -528,6 +528,10 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation | |
/// used when parsing the attribute text. | ||
llvm::SmallDenseMap<ModuleDecl *, SourceFile *> ClangSwiftAttrSourceFiles; | ||
|
||
/// If set, the Swift DeclContext from which the current | ||
/// import request was triggered. | ||
llvm::Optional<const swift::DeclContext *> ImportingContext; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the purposes of this patch, all that matters is that the provided |
||
|
||
public: | ||
/// The Swift lookup table for the bridging header. | ||
std::unique_ptr<SwiftLookupTable> BridgingHeaderLookupTable; | ||
|
@@ -587,6 +591,16 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation | |
return Instance.get(); | ||
} | ||
|
||
void setImportingContext(const DeclContext &importingContext) { | ||
ImportingContext = &importingContext; | ||
} | ||
|
||
void resetImportingContext() { ImportingContext = None; } | ||
|
||
llvm::Optional<const DeclContext *> getImportingContext() { | ||
return ImportingContext; | ||
} | ||
|
||
private: | ||
/// Generation number that is used for crude versioning. | ||
/// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import Foundation | ||
|
||
@objc public class Foo : NSObject { | ||
@objc public func sayHello() { | ||
print("Hello from Foo.sayHello!") | ||
} | ||
} | ||
|
||
@objc(Baz) public class Bar : NSObject { | ||
@objc public func sayHello() { | ||
print("Hello from Bar.sayHello!") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module ObjCLibraryForwardDeclaringCompleteSwiftTypes { | ||
header "objc-library-forward-declaring-complete-swift-types.h" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
@class Foo; | ||
@class Baz; | ||
|
||
void takeAFoo(Foo *foo); | ||
Foo *returnAFoo(); | ||
|
||
void takeABaz(Baz *baz); | ||
Baz *returnABaz(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#import "objc-library-forward-declaring-complete-swift-types.h" | ||
#import "CompleteSwiftTypes-Swift.h" | ||
|
||
void takeAFoo(Foo *foo) { [foo sayHello]; } | ||
|
||
Foo *returnAFoo() { | ||
Foo *result = [[Foo alloc] init]; | ||
[result sayHello]; | ||
return result; | ||
} | ||
|
||
void takeABaz(Baz *baz) { [baz sayHello]; } | ||
|
||
Baz *returnABaz() { | ||
Baz *result = [[Baz alloc] init]; | ||
[result sayHello]; | ||
return result; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
// RUN: %empty-directory(%t) | ||
// RUN: %target-build-swift -parse-as-library %S/Inputs/custom-modules/IncompleteTypes/complete-swift-types.swift -emit-module -emit-module-path %t/CompleteSwiftTypes.swiftmodule -emit-objc-header -emit-objc-header-path %t/CompleteSwiftTypes-Swift.h -emit-library -o %t/libCompleteSwiftTypes.dylib | ||
// RUN: %target-clang -framework Foundation -dynamiclib %S/Inputs/custom-modules/IncompleteTypes/objc-library-forward-declaring-complete-swift-types.m -I %t -L %t -lCompleteSwiftTypes -o %t/libObjCLibraryForwardDeclaringCompleteSwiftTypes.dylib | ||
// RUN: %target-build-swift -Xfrontend -enable-objc-interop %s -I %S/Inputs/custom-modules/IncompleteTypes -I %t -L %t -lCompleteSwiftTypes -lObjCLibraryForwardDeclaringCompleteSwiftTypes -o %t/a.out | ||
// RUN: %target-run %t/a.out | %FileCheck %s | ||
|
||
// REQUIRES: objc_interop | ||
|
||
import CompleteSwiftTypes | ||
import ObjCLibraryForwardDeclaringCompleteSwiftTypes | ||
|
||
// Swift initializers | ||
let foo = Foo() | ||
let bar = Bar() | ||
|
||
// Imported from Objective-C | ||
// CHECK: Hello from Foo.sayHello! | ||
takeAFoo(foo) | ||
// CHECK: Hello from Foo.sayHello! | ||
let foo2 = returnAFoo() | ||
|
||
// CHECK: Hello from Bar.sayHello! | ||
takeABaz(bar) | ||
// CHECK: Hello from Bar.sayHello! | ||
let bar2 = returnABaz() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fallback was kept around so as to not break source compatibility, but it was never intended to work and should probably stop working in the next major version: #27921