Skip to content

Conversation

calda
Copy link
Contributor

@calda calda commented Dec 22, 2021

This PR adds support for if let foo { optional binding conditions.

This implements SE-0345.

For example:

var foo: String? = "hello world"

if let foo {
  // `foo` is unwrapped to `String`
  print(foo) // prints "hello world"
}

@DougGregor
Copy link
Member

@swift-ci please smoke test

@DougGregor
Copy link
Member

@swift-ci please built toolchain

@DougGregor
Copy link
Member

Could you add some tests into, e.g., test/stmt/if_while_var.swift, to show that this is doing the right thing in both well-formed cases (the x in if x { ... } is available and optional) and ill-formed cases (x doesn't exist, x isn't optional, etc.`)?

@calda
Copy link
Contributor Author

calda commented Jan 4, 2022

Of course, will work on this tonight / this week. Thanks for the pointer of what tests to update!

@calda calda force-pushed the cal--if-let-shorthand branch from ee8ebc0 to b9c2370 Compare January 4, 2022 17:36
@calda
Copy link
Contributor Author

calda commented Jan 4, 2022

@DougGregor, added tests in 38e2b70

@calda calda force-pushed the cal--if-let-shorthand branch 3 times, most recently from 6cd6002 to 362234d Compare January 5, 2022 05:39
@calda calda force-pushed the cal--if-let-shorthand branch from 362234d to 38e2b70 Compare January 5, 2022 14:25
@DougGregor
Copy link
Member

@swift-ci please build toolchain

@DougGregor
Copy link
Member

@swift-ci please build toolchain

@DougGregor
Copy link
Member

@swift-ci please smoke test

@AnthonyLatsis
Copy link
Collaborator

AnthonyLatsis commented Mar 23, 2022

Could you also add some tests for the following?

  • if let x {} where x is not defined.
  • if let x: Int {} at least for a matching wrapped type.
  • if case let .y(x) {}, if case .y(let x) {}, if case let x?.

@calda
Copy link
Contributor Author

calda commented Mar 24, 2022

Updated in cc85edf

@DougGregor
Copy link
Member

@swift-ci please smoke test

@DougGregor
Copy link
Member

SE-0345 has been accepted. Kicking off CI to check that we're in a mergeable state. One thing we'll still want to add is a ChangeLog entry describing the change. That can come as a separate PR, of course

@calda calda changed the title Support if let foo { optional binding conditions [SE-0345] Support if let foo { optional binding conditions Mar 31, 2022
@DougGregor DougGregor merged commit a6bcd80 into swiftlang:main Mar 31, 2022
@calda
Copy link
Contributor Author

calda commented Mar 31, 2022

Woohoo! Thanks for merging @DougGregor, appreciate your leadership as RM throughout the review 😄

ktoso added a commit to ktoso/swift that referenced this pull request Jun 17, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This patch "peeks" at the expression to notice this case, however
perhaps we should mark this implicit expression in ParseStmt and then
detect it explicitly.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 17, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This patch "peeks" at the expression to notice this case, however
perhaps we should mark this implicit expression in ParseStmt and then
detect it explicitly.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 17, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This patch "peeks" at the expression to notice this case, however
perhaps we should mark this implicit expression in ParseStmt and then
detect it explicitly.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 17, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This patch "peeks" at the expression to notice this case, however
perhaps we should mark this implicit expression in ParseStmt and then
detect it explicitly.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 18, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This patch "peeks" at the expression to notice this case, however
perhaps we should mark this implicit expression in ParseStmt and then
detect it explicitly.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 18, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

Since the synthesized node does have proper source location, we can
avoid this problem easily if we dont mark it as Implicit. Other
approaches are possible but end up in a whack-a-mole between a lot of
other error reporting in other situations which do rely on implicit code
not being reported because they report on it already in some other
specialized way.

Resolves rdar://126169564

what if we always diagnose

Propsing we use this shape of fix for the async if let in synchronous code

This change is that we don't mark the implicit declref code emitted by
the `if let prop` as implicit anymore, and this way the reporting works
out as expected.

Removing the if (isImplicit) return-early is actually tricky since
there's quite a whack-a-mole with errors getting reported then...

This change looks the safest to me.
ktoso added a commit to ktoso/swift that referenced this pull request Jun 18, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

Since the synthesized node does have proper source location, we can
avoid this problem easily if we dont mark it as Implicit. Other
approaches are possible but end up in a whack-a-mole between a lot of
other error reporting in other situations which do rely on implicit code
not being reported because they report on it already in some other
specialized way.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 19, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This change is that we don't mark the implicit declref code emitted by
the `if let prop` as implicit anymore, and this way the reporting works
out as expected.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 19, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This change is that we don't mark the implicit declref code emitted by
the `if let prop` as implicit anymore, and this way the reporting works
out as expected.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso added a commit to ktoso/swift that referenced this pull request Jun 19, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: swiftlang#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This change is that we don't mark the implicit declref code emitted by
the `if let prop` as implicit anymore, and this way the reporting works
out as expected.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
ktoso referenced this pull request in ktoso/swift Jun 19, 2024
The issue is that the shorthand if let syntax injects an implicit
expression: apple#40694 in ParseStmt and
that the 'diagnoseUnhandledAsyncSite' explicitly avoids reporting errors
in implicit expressions.

This change is that we don't mark the implicit declref code emitted by
the `if let prop` as implicit anymore, and this way the reporting works
out as expected.

Added some tests covering this as well as properly erroring out for the
nonexistent syntax of shortand + awaiting which doesn't exist, and we
properly error on it.

Resolves rdar://126169564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants