Skip to content

[region-isolation] Do not ignore non-trivial results that are Sendable to be more permissive in the face of lazy typechecker issues. #75541

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
19 changes: 12 additions & 7 deletions lib/SILOptimizer/Analysis/RegionAnalysis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2891,9 +2891,6 @@ CONSTANT_TRANSLATION(VectorInst, Asserting)

#define IGNORE_IF_SENDABLE_RESULT_ASSIGN_OTHERWISE(INST) \
TranslationSemantics PartitionOpTranslator::visit##INST(INST *inst) { \
if (!isNonSendableType(inst->getType())) { \
return TranslationSemantics::Ignored; \
} \
return TranslationSemantics::Assign; \
}

Expand Down Expand Up @@ -2922,10 +2919,18 @@ IGNORE_IF_SENDABLE_RESULT_ASSIGN_OTHERWISE(StructExtractInst)
return TranslationSemantics::Require; \
} \
\
if (isResultNonSendable) \
return TranslationSemantics::AssignFresh; \
\
return TranslationSemantics::Ignored; \
/* We always use assign fresh here regardless of whether or not a */ \
/* type is sendable. */ \
/* */ \
/* DISCUSSION: Since the typechecker is lazy, if there is a bug */ \
/* that causes us to not look up enough type information it is */ \
/* possible for a type to move from being Sendable to being */ \
/* non-Sendable. For example, we could call isNonSendableType and */ \
/* get that the type is non-Sendable, but as a result of calling */ \
/* that API, the type checker could get more information that the */ \
/* next time we call isNonSendableType, we will get that the type */ \
/* is non-Sendable. */ \
return TranslationSemantics::AssignFresh; \
}

LOOKTHROUGH_IF_NONSENDABLE_RESULT_AND_OPERAND(UncheckedTrivialBitCastInst)
Expand Down
12 changes: 6 additions & 6 deletions test/Concurrency/transfernonsendable.swift
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,8 @@ func letSendableTrivialLetStructFieldTest() async {
await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
// expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
// expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
_ = test.letSendableTrivial
useValue(test) // expected-tns-note {{access can happen concurrently}}
_ = test.letSendableTrivial // expected-tns-note {{access can happen concurrently}}
useValue(test)
}

func letSendableNonTrivialLetStructFieldTest() async {
Expand Down Expand Up @@ -947,20 +947,20 @@ func letNonSendableNonTrivialLetStructFieldClosureTest() async {
await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
// expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
// expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
let z = test.letSendableNonTrivial
let z = test.letSendableNonTrivial // expected-tns-note {{access can happen concurrently}}
_ = z
let z2 = test.varSendableNonTrivial
_ = z2
useValue(test) // expected-tns-note {{access can happen concurrently}}
useValue(test)
}

func varSendableTrivialLetStructFieldTest() async {
let test = StructFieldTests()
await transferToMain(test) // expected-tns-warning {{sending 'test' risks causing data races}}
// expected-tns-note @-1 {{sending 'test' to main actor-isolated global function 'transferToMain' risks causing data races between main actor-isolated and local nonisolated uses}}
// expected-complete-warning @-2 {{passing argument of non-sendable type 'StructFieldTests' into main actor-isolated context may introduce data races}}
_ = test.varSendableTrivial
useValue(test) // expected-tns-note {{access can happen concurrently}}
_ = test.varSendableTrivial // expected-tns-note {{access can happen concurrently}}
useValue(test)
}

func varSendableNonTrivialLetStructFieldTest() async {
Expand Down
84 changes: 84 additions & 0 deletions test/Concurrency/transfernonsendable_typecheckerbugs.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t/src)
// RUN: %empty-directory(%t/sdk)
// RUN: %empty-directory(%t/sdk/ObjCAPI)
// RUN: %empty-directory(%t/sdk/SwiftAPI)
// RUN: %empty-directory(%t/compile)
// RUN: split-file %s %t/src

// Build Objective-C lib
// RUN: %target-clang -dynamiclib %t/src/ObjCAPI.m -I %t/src -o %t/sdk/ObjCAPI/libObjCAPI.dylib -lobjc
// RUN: cp %t/src/ObjCAPI.modulemap %t/sdk/ObjCAPI/module.modulemap
// RUN: cp %t/src/ObjCAPI.h %t/sdk/ObjCAPI/ObjCAPI.h

// Build the swift part of API
// RUN: %target-swiftc_driver -emit-library -emit-module-path %t/sdk/SwiftAPI/SwiftAPI.swiftmodule %t/src/SwiftAPI.swift -parse-as-library -I %t/sdk -swift-version 6 -module-name SwiftAPI -enable-library-evolution

// Now compile against the API.
// RUN: %target-swift-frontend -emit-sil -o /dev/null -I %t/sdk/SwiftAPI -I %t/sdk/ObjCAPI %t/src/main.swift -swift-version 6

// REQUIRES: asserts
// REQUIRES: concurrency
// REQUIRES: objc_interop

// Test

//--- ObjCAPI.modulemap

module ObjCAPI {
header "ObjCAPI.h"
}

//--- ObjCAPI.h

#define NS_SWIFT_SENDABLE __attribute__((swift_attr("@Sendable")))
#define NS_SWIFT_NONSENDABLE __attribute__((swift_attr("@_nonSendable")))

NS_SWIFT_NONSENDABLE
@interface MyParentClass
@end

NS_SWIFT_SENDABLE
@interface MySubClass : MyParentClass
@end

//--- ObjCAPI.m

#include "ObjCAPI.h"

@implementation MyParentClass
@end

@implementation MySubClass
@end

//--- SwiftAPI.swift

@_exported import ObjCAPI // Clang module

public struct Measurement<T : MyParentClass> {
/// The unit component of the `Measurement`.
public let unit: T

public var count: Int { 0 }
}

extension Measurement : Sendable where T : Sendable {}

//--- main.swift

public import SwiftAPI

public enum MarketplaceKitError : Sendable {
case first
case second(Measurement<MySubClass>)

public var description: String {
switch self {
case .first:
return "first"
case .second(let value):
return "\(value.count)"
}
}
}