Skip to content

AST: Fix accessibility checking in opaque type archetype substitution logic #30847

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
18 changes: 11 additions & 7 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2932,13 +2932,13 @@ substOpaqueTypesWithUnderlyingTypes(Type ty, const DeclContext *inContext,
static bool canSubstituteTypeInto(Type ty, const DeclContext *dc,
OpaqueSubstitutionKind kind,
bool isContextWholeModule) {
ValueDecl *nominal = ty->getAnyNominal();
if (!nominal) {
TypeDecl *typeDecl = ty->getAnyNominal();
if (!typeDecl) {
// We also need to check that the opaque type descriptor is accessible.
if (auto opaqueTy = ty->getAs<OpaqueTypeArchetypeType>())
nominal = opaqueTy->getDecl();
typeDecl = opaqueTy->getDecl();
}
if (!nominal) {
if (!typeDecl) {
return true;
}

Expand All @@ -2956,14 +2956,18 @@ static bool canSubstituteTypeInto(Type ty, const DeclContext *dc,

// In the same file any visibility is okay.
if (!dc->isModuleContext() &&
nominal->getDeclContext()->getParentSourceFile() ==
typeDecl->getDeclContext()->getParentSourceFile() ==
dc->getParentSourceFile())
return true;
return nominal->getEffectiveAccess() > AccessLevel::FilePrivate;

return typeDecl->getEffectiveAccess() > AccessLevel::FilePrivate;

case OpaqueSubstitutionKind::SubstituteNonResilientModule:
// Can't access types that are not public from a different module.
return nominal->getEffectiveAccess() > AccessLevel::Internal;
if (dc->getParentModule() == typeDecl->getDeclContext()->getParentModule())
return true;

return typeDecl->getEffectiveAccess() > AccessLevel::Internal;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this! Is there an existing resilience API we can use here to avoid duplicating visibility logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None that I'm aware of at the moment. I have a follow-up PR with a couple of cleanups for a few other things I noticed while looking around at opaque type lowering, but not this function in particular. It's something we should chip away at over time.

}
}

Expand Down
29 changes: 29 additions & 0 deletions test/SILGen/Inputs/opaque_result_type_fragile_other.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
public protocol View {}

struct InternalView : View {}
struct InternalGenericView<T> : View {}

public struct PublicView : View {}
public struct PublicGenericView<T> : View {}

extension View {
public func passThrough() -> some View {
return self
}

public func wrapWithInternalView() -> some View {
return InternalView()
}

public func wrapWithInternalGenericView() -> some View {
return InternalGenericView<Self>()
}

public func wrapWithPublicView() -> some View {
return PublicView()
}

public func wrapWithPublicGenericView() -> some View {
return PublicGenericView<Self>()
}
}
46 changes: 46 additions & 0 deletions test/SILGen/opaque_result_type_fragile.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// RUN: %empty-directory(%t)
// RUN: %target-swift-frontend -disable-availability-checking -emit-module %S/Inputs/opaque_result_type_fragile_other.swift -emit-module-path %t/opaque_result_type_fragile_other.swiftmodule
// RUN: %target-swift-frontend -disable-availability-checking -emit-silgen -I%t %s | %FileCheck %s

import opaque_result_type_fragile_other

struct InternalView: View {}
public struct PublicView: View {}

public func testInternalView() {
let v = InternalView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE11passThroughQryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out τ_0_0
_ = v.passThrough()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE016wrapWithInternalF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s32opaque_result_type_fragile_other4ViewPAAE016wrapWithInternalF0QryF", 0) 🦸<τ_0_0>
_ = v.wrapWithInternalView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE023wrapWithInternalGenericF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s32opaque_result_type_fragile_other4ViewPAAE023wrapWithInternalGenericF0QryF", 0) 🦸<τ_0_0>
_ = v.wrapWithInternalGenericView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE014wrapWithPublicF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out PublicView
_ = v.wrapWithPublicView()

//CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE021wrapWithPublicGenericF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out PublicGenericView<τ_0_0>
_ = v.wrapWithPublicGenericView()
}

public func testPublicView() {
let v = PublicView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE11passThroughQryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out τ_0_0
_ = v.passThrough()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE016wrapWithInternalF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s32opaque_result_type_fragile_other4ViewPAAE016wrapWithInternalF0QryF", 0) 🦸<τ_0_0>
_ = v.wrapWithInternalView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE023wrapWithInternalGenericF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out @_opaqueReturnTypeOf("$s32opaque_result_type_fragile_other4ViewPAAE023wrapWithInternalGenericF0QryF", 0) 🦸<τ_0_0>
_ = v.wrapWithInternalGenericView()

// CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE014wrapWithPublicF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out PublicView
_ = v.wrapWithPublicView()

//CHECK: function_ref @$s32opaque_result_type_fragile_other4ViewPAAE021wrapWithPublicGenericF0QryF : $@convention(method) <τ_0_0 where τ_0_0 : View> (@in_guaranteed τ_0_0) -> @out PublicGenericView<τ_0_0>
_ = v.wrapWithPublicGenericView()
}
25 changes: 2 additions & 23 deletions test/SILOptimizer/cast_folding.swift
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
// RUN: %target-swift-frontend -disable-availability-checking -O -emit-sil %s | %FileCheck %s
// RUN: %target-swift-frontend -disable-availability-checking -Onone -emit-sil %s | %FileCheck %s --check-prefix=MANDATORY
// RUN: %target-swift-frontend -Xllvm -sil-disable-pass=FunctionSignatureOpts -Xllvm -sil-disable-pass=PerfInliner -enable-ownership-stripping-after-serialization -disable-availability-checking -O -emit-sil %s | %FileCheck %s
// RUN: %target-swift-frontend -enable-ownership-stripping-after-serialization -disable-availability-checking -Onone -emit-sil %s | %FileCheck %s --check-prefix=MANDATORY
// RUN: %target-swift-frontend -O -emit-sil %s | %FileCheck %s
// RUN: %target-swift-frontend -Xllvm -sil-disable-pass=FunctionSignatureOpts -Xllvm -sil-disable-pass=PerfInliner -enable-ownership-stripping-after-serialization -O -emit-sil %s | %FileCheck %s

// We want to check two things here:
// - Correctness
Expand Down Expand Up @@ -1069,25 +1067,6 @@ public func testCastToPForOptionalFailure() -> Bool {
return testCastToPForOptional(t)
}

struct Underlying : P {
}

public func returnOpaque() -> some P {
return Underlying()
}

// MANDATORY-LABEL: sil{{.*}} @$s12cast_folding23testCastOpaqueArchetypeyyF
// MANDATORY: [[O:%.*]] = alloc_stack $@_opaqueReturnTypeOf("$s12cast_folding12returnOpaqueQryF", 0)
// MANDATORY: [[F:%.*]] = function_ref @$s12cast_folding12returnOpaqueQryF
// MANDATORY: apply [[F]]([[O]])
// MANDATORY: [[U:%.*]] = alloc_stack $Underlying
// MANDATORY: unconditional_checked_cast_addr @_opaqueReturnTypeOf{{.*}}in [[O]] : $*@_opaqueReturnTypeOf{{.*}}to Underlying in [[U]] : $*Underlying
// MANDATORY: load [[U]] : $*Underlying
@inlinable
public func testCastOpaqueArchetype() {
let o = returnOpaque() as! Underlying
}

print("test0=\(test0())")

print("test1=\(test1())")
Expand Down
21 changes: 21 additions & 0 deletions test/SILOptimizer/cast_folding_opaque.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// RUN: %target-swift-frontend -enable-library-evolution -disable-availability-checking -O -emit-sil %s
// RUN: %target-swift-frontend -enable-library-evolution -disable-availability-checking -Onone -emit-sil %s | %FileCheck %s

public protocol P {}

public struct Underlying : P {
}

public func returnOpaque() -> some P {
return Underlying()
}

// CHECK-LABEL: sil [serialized] @$s19cast_folding_opaque23testCastOpaqueArchetypeAA10UnderlyingVyF
// CHECK: [[O:%.*]] = alloc_stack $@_opaqueReturnTypeOf("$s19cast_folding_opaque12returnOpaqueQryF", 0)
// CHECK: [[F:%.*]] = function_ref @$s19cast_folding_opaque12returnOpaqueQryF
// CHECK: apply [[F]]([[O]])
// CHECK: unconditional_checked_cast_addr @_opaqueReturnTypeOf{{.*}}in [[O]] : $*@_opaqueReturnTypeOf{{.*}}to Underlying in %0 : $*Underlying
@inlinable
public func testCastOpaqueArchetype() -> Underlying {
return returnOpaque() as! Underlying
}