Skip to content

Revert "AST: Filter out some Obj-C overrides when MemberImportVisibility is enabled." #80975

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

Merged
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
40 changes: 7 additions & 33 deletions lib/AST/NameLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2395,8 +2395,6 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
ValueDecl *decl,
bool onlyCompleteObjectInits,
bool requireImport) {
auto &ctx = dc->getASTContext();

// Filter out designated initializers, if requested.
if (onlyCompleteObjectInits) {
if (auto ctor = dyn_cast<ConstructorDecl>(decl)) {
Expand All @@ -2414,43 +2412,19 @@ static bool isAcceptableLookupResult(const DeclContext *dc, NLOptions options,
}

// Check access.
if (!(options & NL_IgnoreAccessControl) && !ctx.isAccessControlDisabled()) {
if (!(options & NL_IgnoreAccessControl) &&
!dc->getASTContext().isAccessControlDisabled()) {
bool allowUsableFromInline = options & NL_IncludeUsableFromInline;
if (!decl->isAccessibleFrom(dc, /*forConformance*/ false,
allowUsableFromInline))
return false;
}

if (requireImport) {
// Check that there is some import in the originating context that makes
// this decl visible.
if (!(options & NL_IgnoreMissingImports)) {
if (!dc->isDeclImported(decl))
return false;
}

// Unlike in Swift, Obj-C allows method overrides to be declared in
// extensions (categories), even outside of the module that defines the
// type that is being extended. When MemberImportVisibility is enabled,
// if these overrides are not filtered out they can hijack name
// lookup and cause the compiler to insist that the module that defines
// the extension be imported, contrary to developer expectations.
//
// Filter results belonging to these extensions out, even when ignoring
// missing imports, if we're in a context that requires imports to access
// member declarations.
if (decl->getOverriddenDecl()) {
if (auto *extension = dyn_cast<ExtensionDecl>(decl->getDeclContext())) {
if (auto *nominal = extension->getExtendedNominal()) {
auto extensionMod = extension->getModuleContext();
auto nominalMod = nominal->getModuleContext();
if (!extensionMod->isSameModuleLookingThroughOverlays(nominalMod) &&
!dc->isDeclImported(extension))
return false;
}
}
}
}
// Check that there is some import in the originating context that makes this
// decl visible.
if (requireImport && !(options & NL_IgnoreMissingImports))
if (!dc->isDeclImported(decl))
return false;

// Check that it has the appropriate ABI role.
if (!ABIRoleInfo(decl).matchesOptions(options))
Expand Down
9 changes: 1 addition & 8 deletions test/NameLookup/Inputs/MemberImportVisibility/Categories_A.h
Original file line number Diff line number Diff line change
@@ -1,13 +1,6 @@
@import Foundation;

@interface Base
- (void)overriddenInOverlayForA __attribute__((deprecated("Categories_A.h")));
- (void)overriddenInOverlayForB __attribute__((deprecated("Categories_A.h")));
- (void)overriddenInOverlayForC __attribute__((deprecated("Categories_A.h")));
- (void)overriddenInSubclassInOverlayForC __attribute__((deprecated("Categories_A.h")));
@end

@interface X : Base
@interface X
@end

@interface X (A)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@
extension X {
public func fromOverlayForA() {}
@objc public func fromOverlayForAObjC() {}

@available(*, deprecated, message: "Categories_A.swift")
public override func overriddenInOverlayForA() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,4 @@
extension X {
public func fromOverlayForB() {}
@objc public func fromOverlayForBObjC() {}

@available(*, deprecated, message: "Categories_B.swift")
public override func overriddenInOverlayForB() {}
}
4 changes: 0 additions & 4 deletions test/NameLookup/Inputs/MemberImportVisibility/Categories_C.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,3 @@
@interface X (C)
- (void)fromC;
@end

@interface SubclassFromC : X
- (instancetype)init;
@end
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,4 @@
extension X {
public func fromOverlayForC() {}
@objc public func fromOverlayForCObjC() {}

@available(*, deprecated, message: "Categories_C.swift")
public override func overriddenInOverlayForC() {}
}

extension SubclassFromC {
@available(*, deprecated, message: "Categories_C.swift")
public override func overriddenInSubclassInOverlayForC() {}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,2 @@
import Categories_C
import Categories_D.Submodule

public func makeSubclassFromC() -> SubclassFromC { SubclassFromC() }

This file was deleted.

This file was deleted.

14 changes: 0 additions & 14 deletions test/NameLookup/Inputs/MemberImportVisibility/ObjCOverloads/Leaf.h

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ public enum EnumInA {
}

open class BaseClassInA {
public init() {}
open func methodInA() {}
open func overriddenMethod() {}
}

public protocol ProtocolInA {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ package enum EnumInB_package {

open class DerivedClassInB: BaseClassInA {
open func methodInB() {}
open override func overriddenMethod() {}
}

extension ProtocolInA {
Expand Down
2 changes: 0 additions & 2 deletions test/NameLookup/Inputs/MemberImportVisibility/members_C.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ public enum EnumInC {

open class DerivedClassInC: DerivedClassInB {
open func methodInC() {}
open override func overriddenMethod() {}
public func asDerivedClassInB() -> DerivedClassInB { return self }
}

extension ProtocolInA {
Expand Down
24 changes: 5 additions & 19 deletions test/NameLookup/members_transitive.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
// REQUIRES: swift_feature_MemberImportVisibility

import members_C
// expected-member-visibility-note 18{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}
// expected-member-visibility-note 16{{add import of module 'members_B'}}{{1-1=internal import members_B\n}}


func testExtensionMembers(x: X, y: Y<Z>) {
Expand Down Expand Up @@ -96,22 +96,8 @@ class DerivedFromClassInC: DerivedClassInC {

struct ConformsToProtocolInA: ProtocolInA {} // expected-member-visibility-error{{type 'ConformsToProtocolInA' does not conform to protocol 'ProtocolInA'}} expected-member-visibility-note {{add stubs for conformance}}

func testInheritedMethods(
a: BaseClassInA,
c: DerivedClassInC,
) {
let b = c.asDerivedClassInB()

a.methodInA()
b.methodInA()
c.methodInA()

b.methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
c.methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}

c.methodInC()

a.overriddenMethod()
b.overriddenMethod() // expected-member-visibility-error{{instance method 'overriddenMethod()' is not available due to missing import of defining module 'members_B'}}
c.overriddenMethod()
func testDerivedMethodAccess() {
DerivedClassInC().methodInC()
DerivedClassInC().methodInB() // expected-member-visibility-error{{instance method 'methodInB()' is not available due to missing import of defining module 'members_B'}}
DerivedFromClassInC().methodInB()
}
19 changes: 3 additions & 16 deletions test/NameLookup/members_transitive_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_B.swift
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_C.swift
// RUN: %target-swift-frontend -emit-module -I %t -I %S/Inputs/MemberImportVisibility -o %t %S/Inputs/MemberImportVisibility/Categories_E.swift
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5 -verify-additional-prefix no-member-visibility-
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 6 -verify-additional-prefix no-member-visibility-
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 6
// RUN: %target-swift-frontend -typecheck %s -I %t -I %S/Inputs/MemberImportVisibility -import-objc-header %S/Inputs/MemberImportVisibility/Bridging.h -verify -swift-version 5 -enable-upcoming-feature MemberImportVisibility -verify-additional-prefix member-visibility-

// REQUIRES: objc_interop
Expand All @@ -13,43 +13,30 @@
import Categories_B
import Categories_E

// expected-member-visibility-note@-1 3 {{add import of module 'Categories_C'}}{{1-1=internal import Categories_C\n}}
// expected-member-visibility-note@-1 2 {{add import of module 'Categories_C'}}{{1-1=internal import Categories_C\n}}
// expected-member-visibility-note@-2 {{add import of module 'Categories_D'}}{{1-1=internal import Categories_D\n}}
func test(x: X) {
x.fromA()
x.fromOverlayForA()
x.overriddenInOverlayForA() // expected-warning {{'overriddenInOverlayForA()' is deprecated: Categories_A.swift}}
x.fromB()
x.fromOverlayForB()
x.overriddenInOverlayForB() // expected-warning {{'overriddenInOverlayForB()' is deprecated: Categories_B.swift}}
x.fromC() // expected-member-visibility-error {{instance method 'fromC()' is not available due to missing import of defining module 'Categories_C'}}
x.fromOverlayForC() // expected-member-visibility-error {{instance method 'fromOverlayForC()' is not available due to missing import of defining module 'Categories_C'}}
x.overriddenInOverlayForC()
// expected-no-member-visibility-warning@-1 {{'overriddenInOverlayForC()' is deprecated: Categories_C.swift}}
// expected-member-visibility-warning@-2 {{'overriddenInOverlayForC()' is deprecated: Categories_A.h}}
x.fromSubmoduleOfD() // expected-member-visibility-error {{instance method 'fromSubmoduleOfD()' is not available due to missing import of defining module 'Categories_D'}}
x.fromBridgingHeader()
x.overridesCategoryMethodOnNSObject()

let subclassFromC = makeSubclassFromC()
subclassFromC.overriddenInSubclassInOverlayForC()
// expected-warning@-1 {{'overriddenInSubclassInOverlayForC()' is deprecated: Categories_C.swift}}
// expected-member-visibility-error@-2 {{instance method 'overriddenInSubclassInOverlayForC()' is not available due to missing import of defining module 'Categories_C'}}
}

func testAnyObject(a: AnyObject) {
a.fromA()
a.fromOverlayForAObjC()
a.overriddenInOverlayForA() // expected-warning {{'overriddenInOverlayForA()' is deprecated: Categories_A.h}}
a.fromB()
a.fromOverlayForBObjC()
a.overriddenInOverlayForB() // expected-warning {{'overriddenInOverlayForB()' is deprecated: Categories_A.h}}
// FIXME: Better diagnostics?
// Name lookup for AnyObject already ignored transitive imports, so
// `MemberImportVisibility` has no effect on these diagnostics.
a.fromC() // expected-error {{value of type 'AnyObject' has no member 'fromC'}}
a.fromOverlayForCObjC() // expected-error {{value of type 'AnyObject' has no member 'fromOverlayForCObjC'}}
a.overriddenInOverlayForC() // expected-warning {{'overriddenInOverlayForC()' is deprecated: Categories_A.h}}
a.fromBridgingHeader()
a.overridesCategoryMethodOnNSObject()
}
Expand Down
Loading