[ClangImporter] Swift Implemented ObjC Forward Declarations #62983
Conversation
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
lib/Serialization/ModuleFile.cpp
Outdated
| @@ -321,6 +321,34 @@ void ModuleFile::lookupValue(DeclName name, | |||
| } | |||
| } | |||
|
|
|||
| void ModuleFile::lookupSwiftValueByObjCName( | |||
| DeclName name, SmallVectorImpl<ValueDecl *> &results) { | |||
| if (Core->TopLevelDecls) { | |||
There was a problem hiding this comment.
Curious how significant the performance penalty of iterating all declarations is / how difficult it is to alter the serialization format to include a table by objc name as well.
|
@swift-ci Please test source compatibility |
|
@swift-ci Please test Windows platform |
NuriAmari
force-pushed
the
swift-forwarding
branch
from
cf97c88 to
2da1c24
Compare
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
| if (found) | ||
| return nullptr; | ||
| found = singleResult; | ||
| if (!languageVersion.isVersionAtLeast(6)) { |
There was a problem hiding this comment.
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
| importingContext.getValue()->getParentSourceFile(); | ||
|
|
||
| // TODO: Make sure we aren't bailing early in legitimate cases | ||
| if (!importingSwiftSourceFile) |
There was a problem hiding this comment.
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.
| @@ -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.
For the purposes of this patch, all that matters is that the provided DeclContext be within the parent SourceFile DeclContext. However, other use cases might require the DeclContext that is the direct parent of the usage? I don't know how easy it will be to pass that information to the lookup in all cases, so I think maybe its best to rename to SourceFileContext, or even just store a SourceFile.
|
@swift-ci Please test |
|
@swift-ci Please test source compatibility |
2 similar comments
|
@swift-ci Please test source compatibility |
|
@swift-ci Please test source compatibility |
Objective-C libraries that wrap Swift types will often forward declare these types in public headers. Prior to this patch, if a Swift client consumed this Obejctive-C header, the forward declarations and dependent declarations will not be imported, despite the client importing the defining Swift module. The same is not true for Clang modules, which was confusing behavior.
This patch allows the ClangImporter to check if a matching @objc Swift declarations is available from the importing Swift context when encountering an incomplete ObjC interface or protocol. If such a Swift declaration is found, it is simply returned.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves #NNNNN, fix apple/llvm-project#MMMMM.