[region-isolation] Implement function sub typing rules #74129

gottesmm · 2024-06-04T22:46:05Z

Just taking care of this real quick.

rdar://127675288

gottesmm · 2024-06-04T22:46:17Z

lib/Sema/CSSimplify.cpp

…witnessed by a function without a sending result. This is important since the witness is not promising to return a disconnected value and the caller of the requirement is going to accept that. rdar://127675288

rdar://127675288

… computeFailureTypeWitness. The problem happens when a type conforms to AsyncIterator.next when next in the protocol returns its value as sending, but the witness does not have sending. This results in the function subtyping rules rejecting the witness... but we still leave in the witness as a nullptr... so we need to handle it here. rdar://129300953

…testing typecheck errors.

…rs and results. I found this while writing tests for the earlier part of this work. Since this is also type checking work, I am just folding this work into that work.

The previous commit fixed things like: ```swift let x: () -> sending String = { "" } ``` This commit fixes this test case: ```swift let x = { () -> sending String in "" } ```

gottesmm · 2024-06-13T00:01:08Z

@swift-ci smoke test

gottesmm · 2024-06-13T06:08:04Z

@swift-ci smoke test

@available

…when not in swift 6. This will ensure that we do not break anyone who has adopted APIs like CheckedContinuation.resume that now have sending parameters. An example of where this can come up is shown by the ProcessType in SwiftToolsCore: ```swift @available(macOS 10.15, iOS 13.0, tvOS 13.0, watchOS 6.0, *) @discardableResult public func waitUntilExit() async throws -> ProcessResult { try await withCheckedThrowingContinuation { continuation in DispatchQueue.processConcurrent.async { self.waitUntilExit(continuation.resume(with:)) } } } ``` This fails to compile since self.waitUntilExit doesn't expect a function that takes a sending parameter. We want to give people time to fix such issues.

gottesmm · 2024-06-14T05:27:26Z

@swift-ci smoke test

ktoso · 2024-06-14T12:09:46Z

test/Concurrency/transfernonsendable_functionsubtyping.swift

+class NonSendableKlass {}
+
+protocol ProtocolWithSendingReqs {
+  func sendingResult() -> sending NonSendableKlass // expected-note {{}}


what is expected note {{}} -- "any note"? First time seeing this tbh; worth spelling it out?

The reason I did this is that the message here isn't important to what I am testing but it is still a diagnostic.

ktoso · 2024-06-14T12:15:10Z

include/swift/Sema/CSFix.h

+  /// parameter.
+  ///
+  /// 2. Where we have a function that expects a function typed parameter with a
+  /// sending result, but is passed a function typeed parameter without a


minor typo:

Suggested change

/// sending result, but is passed a function typeed parameter without a

/// sending result, but is passed a function typed parameter without a

ktoso

Looks OK, I'm not 100% sure about the rule but it sounds good from thinking about it for a while... LGTM

gottesmm · 2024-06-14T16:31:39Z

macOS failure is something unrelated in lldb.

gottesmm · 2024-06-14T16:31:45Z

@swift-ci smoke test macOS platform

For now, just turn off the error in swift 5 mode. I wanted to make this an error but right now I do not have all of the time to do it so this is good in the short term so that people can adopt sending on protocols without breaking people using their framework in executables still compiling in swift 5.

gottesmm · 2024-06-14T16:48:42Z

@swift-ci smoke test

xedin · 2024-06-14T17:31:18Z

include/swift/Sema/CSFix.h

+/// that this is being sent to does not enforce that invariants within its body.
+class AllowSendingMismatch final : public ContextualMismatch {
+public:
+  enum class Kind {


I don't think this is necessary, it could be determined based on the locator associated wit the fix.

xedin · 2024-06-14T17:33:39Z

include/swift/Sema/CSFix.h

+
+  static AllowSendingMismatch *create(ConstraintSystem &cs,
+                                      ConstraintLocator *locator, Type srcType,
+                                      Type dstType, Kind kind);


Nit: Based on all other create methods let's use - cs, src, dst, locator ordering.

xedin · 2024-06-14T17:35:13Z

lib/Sema/CSDiagnostics.h

+  bool diagnoseAsError() override;
+};
+
+class SendingOnFunctionResultMismatchFailure final : public ContextualFailure {


It looks like this split is not necessary, the diagnostic is anchored on a simplified locator so it would point to the right spot.

xedin · 2024-06-14T17:35:34Z

lib/Sema/CSDiagnostics.cpp

+}
+
+bool SendingOnFunctionResultMismatchFailure::diagnoseAsError() {
+  emitDiagnosticAt(getLoc(), diag::sending_function_wrong_sending,


Why not just emitDiagnostic since you are using getLoc() anyway?

xedin · 2024-06-14T17:36:10Z

lib/Sema/CSFix.cpp

+        solution, getFromType(), getToType(), getLocator(), fixBehavior);
+    return failure.diagnose(asNote);
+  }
+  case Kind::Result: {


It doesn't look like this split brings much to the table, the difference could be handled internally to the diagnostic.

ahoppen · 2024-06-17T17:19:34Z

lib/Parse/ParseType.cpp

+  } else if (Tok.isContextualKeyword("sending")) {
+    consumeToken();


Is this correct? My impression is that only parameters can be annotated as sending and not arbitrary types. Looking at this change, I think it might cause the following to stop compiling because sending now gets consumed as a contextual keyword instead of as the type name.

struct sending {} let a: sending = sending() func test(x: sending) {}

@ahoppen I just tested this locally and it isn't an issue.

I just attached a debugger to it and understand why my example didn’t hit the issue that I was thinking about but this one does

struct sending {} func test() { _ = { () -> sending in } }

I think the proper solution here is to check if sending is followed by something that satisfies canParseType. If not, we probably need to backtrack and check if sending itself can be used as the type name.

Didn't need to modify any tests since this shouldn't have any functional change. Just a slight cleanup.

…7a86b82f9d282c Follow up fixes with feedback from #74129

gottesmm requested review from ktoso, AnthonyLatsis, hborla, slavapestov and xedin as code owners June 4, 2024 22:46

xedin approved these changes Jun 4, 2024

View reviewed changes

lib/Sema/CSSimplify.cpp Outdated Show resolved Hide resolved

gottesmm enabled auto-merge June 4, 2024 23:18

gottesmm force-pushed the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch from cbc5c12 to 684a33f Compare June 12, 2024 23:12

gottesmm requested review from ahoppen, bnbarham, CodaFi, DougGregor, hamishknight and rintaro as code owners June 12, 2024 23:12

gottesmm added 5 commits June 12, 2024 16:13

[sending] Do not let protocol function reqs with a sending result be …

1c9b178

…witnessed by a function without a sending result. This is important since the witness is not promising to return a disconnected value and the caller of the requirement is going to accept that. rdar://127675288

[sending] Implement non-protocol function subtyping rules.

3c166b5

rdar://127675288

Simplify test to only use target-typecheck-verify-swift since we are …

ba6c8af

…testing typecheck errors.

[sending] Fix a few bugs around closure inference of sending paramete…

2ac874e

…rs and results. I found this while writing tests for the earlier part of this work. Since this is also type checking work, I am just folding this work into that work.

gottesmm force-pushed the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch from 684a33f to 85a66d7 Compare June 12, 2024 23:13

[sending] Fix type inference for sending results of closures.

81100ad

The previous commit fixed things like: ```swift let x: () -> sending String = { "" } ``` This commit fixes this test case: ```swift let x = { () -> sending String in "" } ```

gottesmm force-pushed the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch from 85a66d7 to 81100ad Compare June 13, 2024 00:00

Fix a test

a451fe6

gottesmm requested a review from adrian-prantl as a code owner June 13, 2024 06:07

gottesmm force-pushed the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch from e8c14fb to 74fa96a Compare June 14, 2024 04:49

gottesmm force-pushed the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch from 74fa96a to 16d0194 Compare June 14, 2024 05:27

gottesmm disabled auto-merge June 14, 2024 05:27

ktoso reviewed Jun 14, 2024

View reviewed changes

ktoso approved these changes Jun 14, 2024

View reviewed changes

gottesmm enabled auto-merge June 14, 2024 16:31

gottesmm mentioned this pull request Jun 14, 2024

[6.0] Fix function subtyping rules for sending #74428

Merged

xedin reviewed Jun 14, 2024

View reviewed changes

gottesmm merged commit f995418 into swiftlang:main Jun 14, 2024
3 checks passed

ahoppen reviewed Jun 17, 2024

View reviewed changes

gottesmm added a commit to gottesmm/swift that referenced this pull request Jun 20, 2024

Follow up fixes with feedback from swiftlang#74129

9b0e0f5

Didn't need to modify any tests since this shouldn't have any functional change. Just a slight cleanup.

gottesmm deleted the pr-d17a3faab1ceab8b831d7649c1005be9c49d771c branch June 20, 2024 03:19

gottesmm added a commit that referenced this pull request Jun 25, 2024

Merge pull request #74564 from gottesmm/pr-9be87019f95149167c2e03043b…

56cb980

…7a86b82f9d282c Follow up fixes with feedback from #74129

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[region-isolation] Implement function sub typing rules #74129

[region-isolation] Implement function sub typing rules #74129

gottesmm commented Jun 4, 2024

gottesmm commented Jun 4, 2024

gottesmm commented Jun 13, 2024

gottesmm commented Jun 13, 2024

gottesmm commented Jun 14, 2024

ktoso Jun 14, 2024

gottesmm Jun 14, 2024

ktoso Jun 14, 2024

ktoso left a comment

gottesmm commented Jun 14, 2024

gottesmm commented Jun 14, 2024

gottesmm commented Jun 14, 2024

xedin Jun 14, 2024

xedin Jun 14, 2024

xedin Jun 14, 2024

xedin Jun 14, 2024

xedin Jun 14, 2024

ahoppen Jun 17, 2024

gottesmm Jun 20, 2024

ahoppen Jun 20, 2024

	/// sending result, but is passed a function typeed parameter without a
	/// sending result, but is passed a function typed parameter without a

		} else if (Tok.isContextualKeyword("sending")) {
		consumeToken();

[region-isolation] Implement function sub typing rules #74129

[region-isolation] Implement function sub typing rules #74129

Conversation

gottesmm commented Jun 4, 2024

gottesmm commented Jun 4, 2024

gottesmm commented Jun 13, 2024

gottesmm commented Jun 13, 2024

gottesmm commented Jun 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ktoso left a comment

Choose a reason for hiding this comment

gottesmm commented Jun 14, 2024

gottesmm commented Jun 14, 2024

gottesmm commented Jun 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment