Skip to content

[BitwiseCopyable] Don't ever infer for public. #71613

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
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
62 changes: 54 additions & 8 deletions lib/SIL/IR/TypeLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ llvm::cl::opt<bool> TypeLoweringForceOpaqueValueLowering(
llvm::cl::desc("Force TypeLowering to behave as if building with opaque "
"values enabled"));

llvm::cl::opt<bool> TypeLoweringDisableVerification(
"type-lowering-disable-verification", llvm::cl::init(false),
llvm::cl::desc("Disable the asserts-only verification of lowerings"));

namespace {
/// A CRTP type visitor for deciding whether the metatype for a type
/// is a singleton type, i.e. whether there can only ever be one
Expand Down Expand Up @@ -2945,6 +2949,9 @@ void TypeConverter::verifyLowering(const TypeLowering &lowering,
AbstractionPattern origType,
CanType substType,
TypeExpansionContext forExpansion) {
if (TypeLoweringDisableVerification) {
return;
}
verifyLexicalLowering(lowering, origType, substType, forExpansion);
verifyTrivialLowering(lowering, origType, substType, forExpansion);
}
Expand Down Expand Up @@ -3049,7 +3056,7 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,

if (lowering.isTrivial() && !conformance) {
// A trivial type can lack a conformance in a few cases:
// (1) containing or being a resilient type
// (1) containing or being a public, non-frozen type
// (2) containing or being a generic type which doesn't conform
// unconditionally but in this particular instantiation is trivial
// (3) being a special type that's not worth forming a conformance for
Expand All @@ -3064,6 +3071,7 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
// unowned(unsafe) var o: AnyObject
// }
// (5) being defined in a different module
// (6) being defined in a module built from interface
bool hasNoNonconformingNode = visitAggregateLeaves(
origType, substType, forExpansion,
/*isLeafAggregate=*/
Expand All @@ -3081,12 +3089,22 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
return true;
}

// Resilient trivial types may not conform (case (1)).
if (nominal->isResilient())
// Public, non-frozen trivial types may not conform (case (1)).
if (nominal
->getFormalAccessScope(/*useDC=*/nullptr,
/*treatUsableFromInlineAsPublic=*/true)
.isPublic())
return true;

auto *module = nominal->getModuleContext();

// Types in modules built from interfaces may not conform (case (6)).
if (module && module->isBuiltFromInterface()) {
return false;
}

// Trivial types from other modules may not conform (case (5)).
return nominal->getModuleContext() != &M;
return module != &M;
},
/*visit=*/
[&](auto ty, auto origTy, auto *field, auto index) -> bool {
Expand Down Expand Up @@ -3141,12 +3159,22 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
return false;
}

// Resilient trivial types may not conform (case (1)).
if (nominal->isResilient())
// Public, non-frozen trivial types may not conform (case (1)).
if (nominal
->getFormalAccessScope(/*useDC=*/nullptr,
/*treatUsableFromInlineAsPublic=*/true)
.isPublic())
return false;

auto *module = nominal->getModuleContext();

// Types in modules built from interfaces may not conform (case (6)).
if (module && module->isBuiltFromInterface()) {
return false;
}

// Trivial types from other modules may not conform (case (5)).
return nominal->getModuleContext() == &M;
return module == &M;
});
if (hasNoNonconformingNode) {
llvm::errs() << "Trivial type without a BitwiseCopyable conformance!?:\n"
Expand All @@ -3159,10 +3187,19 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
if (!lowering.isTrivial() && conformance) {
// A non-trivial type can have a conformance in one case:
// (1) contains a conforming archetype
// (2) is resilient with minimal expansion
bool hasNoConformingArchetypeNode = visitAggregateLeaves(
origType, substType, forExpansion,
/*isLeaf=*/
[&](auto ty, auto origTy, auto *field, auto index) -> bool {
// A resilient type that's with minimal expansion may be non-trivial
// but still conform (case (2)).
auto *nominal = ty.getAnyNominal();
if (nominal && nominal->isResilient() &&
forExpansion.getResilienceExpansion() ==
ResilienceExpansion::Minimal) {
return true;
}
// Walk into every aggregate.
return false;
},
Expand All @@ -3173,7 +3210,16 @@ void TypeConverter::verifyTrivialLowering(const TypeLowering &lowering,
"leaf of non-trivial BitwiseCopyable type that doesn't "
"conform to BitwiseCopyable!?");

// An archetype may conform but be non-trivial (case (2)).
// A resilient type that's with minimal expansion may be non-trivial
// but still conform (case (2)).
auto *nominal = ty.getAnyNominal();
if (nominal && nominal->isResilient() &&
forExpansion.getResilienceExpansion() ==
ResilienceExpansion::Minimal) {
return false;
}

// An archetype may conform but be non-trivial (case (1)).
if (origTy.isTypeParameter())
return false;

Expand Down
3 changes: 1 addition & 2 deletions lib/Sema/TypeCheckBitwise.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,7 @@ getImplicitCheckForNominal(NominalTypeDecl *nominal) {
if (!nominal
->getFormalAccessScope(
/*useDC=*/nullptr, /*treatUsableFromInlineAsPublic=*/true)
.isPublic() ||
!nominal->isResilient())
.isPublic())
return {BitwiseCopyableCheck::Implicit};

if (nominal->hasClangNode() ||
Expand Down
55 changes: 55 additions & 0 deletions test/IRGen/bitwise_copyable_resilient.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

// RUN: %target-swift-frontend \
// RUN: %t/Library.swift \
// RUN: -emit-module \
// RUN: -enable-library-evolution \
// RUN: -enable-experimental-feature BitwiseCopyable \
// RUN: -module-name Library \
// RUN: -emit-module-path %t/Library.swiftmodule

// RUN: %target-swift-frontend \
// RUN: %t/Downstream.swift \
// RUN: -typecheck -verify \
// RUN: -debug-diagnostic-names \
// RUN: -enable-experimental-feature BitwiseCopyable \
// RUN: -I %t

//--- Library.swift
public enum Oopsional<T> {
case someone(T)
case nobody
}

@frozen public enum Woopsional<T> {
case somebody(T)
case noone
}

public struct Integer {
var value: Int
}

//--- Downstream.swift
import Library

func take<T: _BitwiseCopyable>(_ t: T) {}

struct S_Explicit_With_Oopsional<T> : _BitwiseCopyable {
var o: Oopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
}

func passOopsional<T>(_ t: Oopsional<T>) { take(t) } // expected-error {{type_does_not_conform_decl_owner}}
// expected-note@-7 {{where_requirement_failure_one_subst}}


struct S_Explicit_With_Woopsional<T> : _BitwiseCopyable {
var o: Woopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
}

func passWoopsional<T>(_ t: Woopsional<T>) { take(t) } // expected-error {{type_does_not_conform_decl_owner}}
// expected-note@-15 {{where_requirement_failure_one_subst}}

extension Integer : @retroactive _BitwiseCopyable {} // expected-error {{bitwise_copyable_outside_module}}

2 changes: 1 addition & 1 deletion test/SILGen/bitwise_copyable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ enum Context<T> {

func doit() -> Context<Int>.Here { .init(t: 0) }

public enum E {
public enum E : _BitwiseCopyable {
case a
}

Expand Down
3 changes: 2 additions & 1 deletion test/Sema/bitwise_copyable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ struct S_Explicit_With_Function_C : _BitwiseCopyable {
public struct S_Public {}

struct S_Explicit_With_S_Public : _BitwiseCopyable {
var s: S_Public
var s: S_Public // expected-error {{non_bitwise_copyable_type_member}}
// expected-note@-4 {{add_nominal_bitwise_copyable_conformance}}
}

struct S_Explicit_With_Generic<T> : _BitwiseCopyable {
Expand Down
10 changes: 10 additions & 0 deletions test/Sema/bitwise_copyable_2.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,13 @@ indirect enum E_Explicit_Indirect : _BitwiseCopyable { // expected-error {{non_b
enum E_Explicit_Indirect_Case : _BitwiseCopyable { // expected-error {{non_bitwise_copyable_type_indirect_enum_element}}
indirect case s(S) // expected-note {{note_non_bitwise_copyable_type_indirect_enum_element}}
}

func take<T : _BitwiseCopyable>(_ t: T) {}

// public (effectively) => !conforms
@usableFromInline internal struct InternalUsableStruct {
public var int: Int
}

func passInternalUsableStruct(_ s: InternalUsableStruct) { take(s) } // expected-error{{type_does_not_conform_decl_owner}}
// expected-note@-8{{where_requirement_failure_one_subst}}
10 changes: 6 additions & 4 deletions test/Sema/bitwise_copyable_nonresilient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,17 @@ import Library
func take<T: _BitwiseCopyable>(_ t: T) {}

struct S_Explicit_With_Oopsional<T> : _BitwiseCopyable {
var o: Oopsional<T>
var o: Oopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
}

func passOopsional<T>(_ t: Oopsional<T>) { take(t) }
func passOopsional<T>(_ t: Oopsional<T>) { take(t) } // expected-error{{type_does_not_conform_decl_owner}}
// expected-note@-7{{where_requirement_failure_one_subst}}


struct S_Explicit_With_Woopsional<T> : _BitwiseCopyable {
var o: Woopsional<T>
var o: Woopsional<T> // expected-error{{non_bitwise_copyable_type_member}}
}

func passWoopsional<T>(_ t: Woopsional<T>) { take(t) }
func passWoopsional<T>(_ t: Woopsional<T>) { take(t) } // expected-error{{type_does_not_conform_decl_owner}}
// expected-note@-15{{where_requirement_failure_one_subst}}

42 changes: 42 additions & 0 deletions test/Sema/bitwise_copyable_package_resilience.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// RUN: %empty-directory(%t)
// RUN: split-file %s %t

// RUN: %target-swift-frontend \
// RUN: %t/Library.swift \
// RUN: -emit-module \
// RUN: -package-name Package \
// RUN: -enable-library-evolution \
// RUN: -enable-experimental-feature BitwiseCopyable \
// RUN: -module-name Library \
// RUN: -emit-module-path %t/Library.swiftmodule \
// RUN: -emit-module-interface-path %t/Library.swiftinterface

// RUN: %target-swift-frontend \
// RUN: %t/Downstream.swift \
// RUN: -typecheck -verify \
// RUN: -package-name Package \
// RUN: -debug-diagnostic-names \
// RUN: -enable-experimental-feature BitwiseCopyable \
// RUN: -I %t

//--- Library.swift

// !public => conforms
package struct PackageStruct {
package var int: Int
}

// Public => !conforms
public struct PublicStruct {
public var int: Int
}

//--- Downstream.swift
import Library

func take<T : _BitwiseCopyable>(_ t: T) {}

func passPackageStruct(_ s: PackageStruct) { take(s) }

func passPublicStruct(_ s: PublicStruct) { take(s) } // expected-error{{type_does_not_conform_decl_owner}}
// expected-note@-5{{where_requirement_failure_one_subst}}