Skip to content

[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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,12 @@ class ModuleDecl
void lookupValue(DeclName Name, NLKind LookupKind,
SmallVectorImpl<ValueDecl*> &Result) const;

/// Look up a value just as "ModuleDecl::lookupValue` does, but provide the
/// Swift context from which the lookup is made.
void lookupValueWithContext(DeclName Name, NLKind LookupKind,
SmallVectorImpl<ValueDecl *> &Result,
const DeclContext *Context) const;

/// Look up a local type declaration by its mangled name.
///
/// This does a simple local lookup, not recursively looking through imports.
Expand Down
4 changes: 4 additions & 0 deletions include/swift/ClangImporter/ClangModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ class ClangModuleUnit final : public LoadedFile {
virtual void lookupValue(DeclName name, NLKind lookupKind,
SmallVectorImpl<ValueDecl*> &results) const override;

void lookupValueWithContext(DeclName name, NLKind lookupKind,
SmallVectorImpl<ValueDecl *> &results,
const DeclContext *context) const;

virtual TypeDecl *
lookupNestedType(Identifier name,
const NominalTypeDecl *baseType) const override;
Expand Down
27 changes: 26 additions & 1 deletion lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
#include "swift/Basic/Compiler.h"
#include "swift/Basic/SourceManager.h"
#include "swift/Basic/Statistic.h"
#include "swift/ClangImporter/ClangModule.h"
#include "swift/Demangling/ManglingMacros.h"
#include "swift/Parse/Token.h"
#include "swift/Strings.h"
Expand All @@ -57,8 +58,8 @@
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/SaveAndRestore.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"

using namespace swift;

Expand Down Expand Up @@ -736,6 +737,30 @@ void ModuleDecl::lookupValue(DeclName Name, NLKind LookupKind,
FORWARD(lookupValue, (Name, LookupKind, Result));
}

void ModuleDecl::lookupValueWithContext(DeclName Name, NLKind LookupKind,
SmallVectorImpl<ValueDecl *> &Result,
const DeclContext *Context) const {
if (isParsedModule(this)) {
getSourceLookupCache().lookupValue(Name, LookupKind, Result);
return;
}

for (const FileUnit *file : getFiles()) {
// At the time of writing, Clang module units are the only FileUnits
// that make use of the Context. If this changes, add appropriate calls
// below.
if (auto clangModuleUnit = dyn_cast<ClangModuleUnit>(file)) {
clangModuleUnit->lookupValueWithContext(Name, LookupKind, Result,
Context);
} else {
file->lookupValue(Name, LookupKind, Result);
}
if (auto *synth = file->getSynthesizedFile()) {
synth->lookupValue(Name, LookupKind, Result);
}
}
}

TypeDecl * ModuleDecl::lookupLocalType(StringRef MangledName) const {
for (auto file : getFiles()) {
auto TD = file->lookupLocalType(MangledName);
Expand Down
13 changes: 8 additions & 5 deletions lib/AST/ModuleNameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,14 @@ class LookupByName : public ModuleNameLookup<LookupByName> {
}

void doLocalLookup(ModuleDecl *module, ImportPath::Access path,
SmallVectorImpl<ValueDecl *> &localDecls) {
SmallVectorImpl<ValueDecl *> &localDecls,
const DeclContext *callingContext) {
// If this import is specific to some named decl ("import Swift.Int")
// then filter out any lookups that don't match.
if (!path.matches(name))
return;
module->lookupValue(name, lookupKind, localDecls);
module->lookupValueWithContext(name, lookupKind, localDecls,
callingContext);
}
};

Expand All @@ -112,7 +114,8 @@ class LookupVisibleDecls : public ModuleNameLookup<LookupVisibleDecls> {
}

void doLocalLookup(ModuleDecl *module, ImportPath::Access path,
SmallVectorImpl<ValueDecl *> &localDecls) {
SmallVectorImpl<ValueDecl *> &localDecls,
const DeclContext *callingContext) {
VectorDeclConsumer consumer(localDecls);
module->lookupVisibleDecls(path, consumer, lookupKind);
}
Expand Down Expand Up @@ -169,7 +172,7 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(

// Do the lookup into the current module.
auto *module = moduleOrFile->getParentModule();
getDerived()->doLocalLookup(module, accessPath, decls);
getDerived()->doLocalLookup(module, accessPath, decls, moduleOrFile);
updateNewDecls(moduleScopeContext);

bool canReturnEarly = (initialCount != decls.size() &&
Expand All @@ -196,7 +199,7 @@ void ModuleNameLookup<LookupStrategy>::lookupInModule(
return;

getDerived()->doLocalLookup(import.importedModule, import.accessPath,
decls);
decls, moduleOrFile);
updateNewDecls(moduleScopeContext);
};

Expand Down
14 changes: 13 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3286,6 +3286,12 @@ void ClangModuleUnit::getDisplayDecls(SmallVectorImpl<Decl*> &results, bool recu

void ClangModuleUnit::lookupValue(DeclName name, NLKind lookupKind,
SmallVectorImpl<ValueDecl*> &results) const {
lookupValueWithContext(name, lookupKind, results, nullptr);
}

void ClangModuleUnit::lookupValueWithContext(
DeclName name, NLKind lookupKind, SmallVectorImpl<ValueDecl *> &results,
const DeclContext *context) const {
// FIXME: Ignore submodules, which are empty for now.
if (clangModule && clangModule->isSubModule())
return;
Expand All @@ -3305,7 +3311,13 @@ void ClangModuleUnit::lookupValue(DeclName name, NLKind lookupKind,
// Find the corresponding lookup table.
if (auto lookupTable = owner.findLookupTable(clangModule)) {
// Search it.
owner.lookupValue(*lookupTable, name, *consumer);
if (context) {
owner.setImportingContext(*context);
owner.lookupValue(*lookupTable, name, *consumer);
owner.resetImportingContext();
} else {
owner.lookupValue(*lookupTable, name, *consumer);
}
}
}

Expand Down
57 changes: 44 additions & 13 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Copy link
Contributor Author

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

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;
}
}
}
}
Expand Down Expand Up @@ -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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
}

Expand Down
14 changes: 14 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 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.


public:
/// The Swift lookup table for the bridging header.
std::unique_ptr<SwiftLookupTable> BridgingHeaderLookupTable;
Expand Down Expand Up @@ -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.
///
Expand Down
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()