AsyncSequence and protocol conformance rethrows #35224

phausler · 2020-12-24T23:14:31Z

for @airspeedswift to play around with

…orProtocol

…rethrowing verifies appropriately

… function

…ations better

…o be based upon throws and not rethrows spelling

This reverts commit d91f1b25b59fff8e4be107c808895ff3f293b394.

… for await in syntax

phausler · 2021-01-12T19:15:30Z

@swift-ci build toolchain

phausler · 2021-01-12T21:24:17Z

@swift-ci build toolchain

swift-ci · 2021-01-12T22:46:29Z

Linux Toolchain (Ubuntu 16.04)
Download Toolchain
Git Sha - 7431ec4

Install command
tar zxf swift-PR-35224-519-ubuntu16.04.tar.gz
More info

swift-ci · 2021-01-12T23:39:25Z

macOS Toolchain
Download Toolchain
Git Sha - 7431ec4

Install command
tar -zxf swift-PR-35224-820-osx.tar.gz --directory ~/

phausler · 2021-01-15T18:01:54Z

@swift-ci please smoke test

phausler · 2021-01-15T18:04:36Z

10:03:10    '/Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/buildbot_incremental/ninja-build/ninja' '-C' '/Users/buildnode/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/buildbot_incremental/swift-macosx-x86_64' '-t' 'cleandead'
10:03:10 
10:03:10   failed with:
10:03:10 
10:03:10 
   ninja: error: remove(stdlib/public/Darwin/XCTest/OSX/x86_64): Directory not empty

phausler · 2021-01-15T18:04:47Z

@swift-ci please smoke test macOS

phausler · 2021-01-15T19:10:51Z

@swift-ci please smoke test

slavapestov

Looks great!

slavapestov · 2021-01-15T22:31:57Z

include/swift/AST/Attr.def

@@ -359,6 +359,10 @@ SIMPLE_DECL_ATTR(rethrows, Rethrows,
  RejectByParser |
  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,
  57)
+SIMPLE_DECL_ATTR(rethrows, AtRethrows,
+  OnProtocol |
+  ABIStableToAdd | ABIStableToRemove | APIStableToAdd | APIStableToRemove,


I don't think it's API-stable to remove, is it?

hmm good point, yea this is not API stable to remove.

hmm so the previous annotation btw marks regular rethrows API stable to remove, isn't that wrong?

slavapestov · 2021-01-15T22:32:35Z

include/swift/AST/Decl.h

+
+  SWIFT_DEBUG_DUMP;
+
+  friend bool operator==(const ProtocolRethrowsRequirementList &lhs,


I landed some changes that mean request results no longer have to overload == or hash_value(), so you can remove these if you want

slavapestov · 2021-01-15T22:33:17Z

include/swift/AST/DiagnosticsSema.def

@@ -2981,6 +2981,8 @@ ERROR(override_rethrows_with_non_rethrows,none,
      "be 'rethrows'", (bool))
 ERROR(rethrows_without_throwing_parameter,none,
      "'rethrows' function must take a throwing function argument", ())
+ERROR(rethrows_attr_on_non_protocol,none,


This shouldn't be necessary because you annotated the attribute with OnProtocol in Attrs.def

so that means the other code:

void AttributeChecker::visitAtRethrowsAttr(AtRethrowsAttr *attr) { if (isa<ProtocolDecl>(D)) { return; } diagnose(attr->getLocation(), diag::rethrows_attr_on_non_protocol); attr->setInvalid(); }

should change to this?

void AttributeChecker::visitAtRethrowsAttr(AtRethrowsAttr *attr) { }

slavapestov · 2021-01-15T22:33:30Z

include/swift/AST/DiagnosticsSema.def

@@ -4052,6 +4054,8 @@ NOTE(because_rethrows_argument_throws,none,
 NOTE(because_rethrows_default_argument_throws,none,
     "call is to 'rethrows' function, but a defaulted argument function"
     " can throw", ())
+NOTE(because_rethrows_default_conformance_throws,none,
+     "call is to 'rethrows' function, but a conformance can throw", ())


"a conformance has a throwing witness" perhaps?

that will be the first case of witness being used in diagnostics, are we certain that is grok-able enough?
perhaps I can write it as "a conformance has a throwing method satisfying a protocol"?

slavapestov · 2021-01-15T22:33:46Z

include/swift/AST/Requirement.h

@@ -46,6 +46,7 @@ enum class RequirementKind : unsigned {
  // when adding enumerators.
 };

+


Unnecessary blank line?

slavapestov · 2021-01-15T22:45:13Z

lib/Sema/TypeCheckDecl.cpp

+
+  // only allow rethrowing requirements to be determined from marked protocols
+  if (!decl->getAttrs().hasAttribute<swift::AtRethrowsAttr>()) {
+    return ProtocolRethrowsRequirementList(ctx.AllocateCopy(found));


Isn't 'found' empty here? You can avoid the call to AllocateCopy and just return an empty ProtocolRethrowsRequirementList

slavapestov · 2021-01-15T22:46:18Z

lib/Sema/TypeCheckEffects.cpp

+
+  ModuleDecl *getModuleContext() {
+    assert(getKind() == Kind::Function);
+    return TheFunction->getModuleContext();


Let's always use the DC that we're type checking to get the module instead

hmm that isn't used!, deleting it

slavapestov · 2021-01-15T22:46:29Z

lib/Sema/TypeCheckEffects.cpp

  }

-  static AbstractFunction decomposeFunction(Expr *fn) {
+  static AbstractFunction decomposeFunction(Expr *fn, ConcreteDeclRef declRef = ConcreteDeclRef()) {


Nitpick: Line is too long

slavapestov · 2021-01-15T22:48:01Z

lib/Sema/TypeCheckEffects.cpp


 public:
-  explicit AbstractFunction(Kind kind, Expr *fn)
+  explicit AbstractFunction(Kind kind, Expr *fn, ConcreteDeclRef declRef)


Can you not fish out declRef from fn?

slavapestov · 2021-01-15T22:49:13Z

test/attr/attr_rethrows_protocol.swift

@@ -0,0 +1,78 @@
+// RUN: %target-typecheck-verify-swift
+
+@rethrows


Please add a test for @rethrows on a non-protocol

… tests for both

phausler · 2021-01-20T01:42:57Z

@swift-ci please smoke test

phausler · 2021-01-21T20:31:24Z

@swift-ci please smoke test

…a valid value and adjust the for-await-in silgen test to reflect the cancel changes

phausler · 2021-01-22T18:46:27Z

@swift-ci please smoke test

phausler · 2021-01-23T00:52:10Z

/utils/gyb_syntax_support/StmtNodes.py:77:89: E501 line too long (89 > 88 characters)

phausler · 2021-01-25T16:36:56Z

@swift-ci please smoke test

This reverts commit 6e05240.

phausler force-pushed the async_unified2 branch from ee587c5 to 91cdb03 Compare January 11, 2021 21:43

phausler added 23 commits January 12, 2021 10:58

Initial draft of async sequences

1b7e579

Adjust AsyncSequence associated type requirements

a23038a

Add a draft implementation of AsyncSequence and associated functionality

ea8d396

Correct merge damage and rename from GeneratorProtocol to AsyncIterat…

db778e7

…orProtocol

Add AsyncSequence types to the cmake lists

9096c48

Add cancellation support

ae676a8

[DRAFT] Implementation of protocol conformance rethrowing

faa1da7

Account for ASTVerifier passes to ensure throwing and by conformance …

df70e94

…rethrowing verifies appropriately

Remove commented out code

d8cef33

OtherConstructorDeclRefExpr can also be a source of a rethrowing kind…

0bf5969

… function

Re-order the checkApply logic to account for existing throwing calcul…

f3f7b4f

…ations better

Extract rethrowing calculation into smaller functions

c0b9d39

Allow for closures and protocol conformances to contribute to throwing

de8ddbe

Add unit tests for conformance based rethrowing

8e5b1dd

Restrict rethrowing requirements to only protocols marked with @rethrows

92229b6

Correct logic for gating of @rethrows and adjust the determinates t…

e8c4cd0

…o be based upon throws and not rethrows spelling

Attempt to unify the async sequence features together

019d8ad

Reorder try await to latest syntax

10feb16

revert back to the inout diagnosis

d0f2e06

House mutations in local scope

3faaf3c

Revert "House mutations in local scope"

2c8e6b1

This reverts commit d91f1b25b59fff8e4be107c808895ff3f293b394.

Adjust for inout diagnostics and fall back to original mutation strategy

51fec5b

Convert async flag to source locations and add initial try support to…

d682168

… for await in syntax

phausler force-pushed the async_unified2 branch from 91cdb03 to d682168 Compare January 12, 2021 19:00

Fix case typo of MinMax.swift

7431ec4

Handle throwing cases of iteration of async sequences

6a640a2

restore building XCTest

0f2422d

phausler requested review from jckarter and parkera January 15, 2021 22:13

slavapestov reviewed Jan 15, 2021

View reviewed changes

phausler requested a review from gottesmm January 15, 2021 22:55

First wave of feedback fixes

ac18c22

phausler requested a review from airspeedswift January 19, 2021 16:20

phausler added 2 commits January 19, 2021 17:18

Rework constraints checking for async sequence for-try-await-in checking

21ce9b7

Allow testing of for-await-in parsing and silgen testing and add unit…

48d4bb5

… tests for both

phausler added 5 commits January 20, 2021 10:59

Remove async sequence operators for now

a0f6236

Back out cancellation of AsyncIteratorProtocols

d4d9249

Restructure protocol conformance throws checking and cache results

b25ccab

remove some stray whitespaces

1821182

Correct some merge damage

b186a7d

Ensure the throwing determinate for applying for-await-in always has …

e07ad11

…a valid value and adjust the for-await-in silgen test to reflect the cancel changes

phausler mentioned this pull request Jan 22, 2021

Update syntax to for-await-in syntax definitions swiftlang/swift-syntax#256

Merged

Squelch the python linter for line length

dafe88e

phausler merged commit 6e05240 into swiftlang:main Jan 26, 2021

ahoppen added a commit that referenced this pull request Jan 26, 2021

Revert "AsyncSequence and protocol conformance rethrows (#35224)"

ee65f20

This reverts commit 6e05240.

ahoppen mentioned this pull request Jan 26, 2021

Revert "AsyncSequence and protocol conformance rethrows" #35597

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AsyncSequence and protocol conformance rethrows #35224

AsyncSequence and protocol conformance rethrows #35224

phausler commented Dec 24, 2020

phausler commented Jan 12, 2021

phausler commented Jan 12, 2021

swift-ci commented Jan 12, 2021

swift-ci commented Jan 12, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

slavapestov left a comment

slavapestov Jan 15, 2021

phausler Jan 15, 2021

phausler Jan 16, 2021

slavapestov Jan 15, 2021

slavapestov Jan 15, 2021

phausler Jan 16, 2021

slavapestov Jan 15, 2021

phausler Jan 16, 2021

slavapestov Jan 15, 2021

slavapestov Jan 15, 2021

slavapestov Jan 15, 2021

phausler Jan 20, 2021

slavapestov Jan 15, 2021

slavapestov Jan 15, 2021

slavapestov Jan 15, 2021

phausler commented Jan 20, 2021

phausler commented Jan 21, 2021

phausler commented Jan 22, 2021

phausler commented Jan 23, 2021

phausler commented Jan 25, 2021


		SWIFT_DEBUG_DUMP;

		friend bool operator==(const ProtocolRethrowsRequirementList &lhs,

		@@ -46,6 +46,7 @@ enum class RequirementKind : unsigned {
		// when adding enumerators.
		};

		@@ -0,0 +1,78 @@
		// RUN: %target-typecheck-verify-swift

		@rethrows

AsyncSequence and protocol conformance rethrows #35224

AsyncSequence and protocol conformance rethrows #35224

Conversation

phausler commented Dec 24, 2020

phausler commented Jan 12, 2021

phausler commented Jan 12, 2021

swift-ci commented Jan 12, 2021

swift-ci commented Jan 12, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

phausler commented Jan 15, 2021

slavapestov left a 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

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

Choose a reason for hiding this comment

phausler commented Jan 20, 2021

phausler commented Jan 21, 2021

phausler commented Jan 22, 2021

phausler commented Jan 23, 2021

phausler commented Jan 25, 2021