Skip to content

Conversation

@slavapestov
Copy link
Contributor

Sema had some logic to diagnose closures that captured values before their definition point. The analysis was wrong, and a false negative meant that SILGen would hit an assertion when trying to emit a capture of an undefined value.

Instead, let's just change SILGen to diagnose the invalid capture if it sees an undefined value while emitting captures, defining away the crash completely. This requires better tracking of source locations when building capture lists, but is otherwise simpler.

Fixes https://bugs.swift.org/browse/SR-4812, rdar://problem/40600800.

@slavapestov slavapestov force-pushed the fix-forward-capture-analysis branch from a203852 to fc2fd49 Compare June 27, 2019 20:43
@slavapestov
Copy link
Contributor Author

@swift-ci Please test source compatibility

Sema does not have enough information to diagnose these problems correctly.

Also, there was an unimplemented case in the analysis; if a closure did not
yet have computed captures, we'd add it to the ForwardCapturedFuncs list,
but nothing ever looked at that list.
…cation

Otherwise we can't produce good diagnostics pointing at the right
'defer' block.
This will be used to emit diagnostics in SILGen when a value is
captured before being defined.
This was the only case where getDeclCaptureKind() would return
CaptureKind::None, and we can simplify some code by not handling
that case.
The new analysis simply checks if the captured value has been defined
yet; instead of asserting if it hasn't, we can now emit a diagnostic
using the source location tracked in the capture list.

Fixes <https://bugs.swift.org/browse/SR-4812>, <rdar://problem/40600800>.
@slavapestov slavapestov force-pushed the fix-forward-capture-analysis branch from fc2fd49 to 44142cd Compare June 28, 2019 01:14
@slavapestov
Copy link
Contributor Author

apple/swift-lldb#1749
@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

apple/swift-lldb#1749
@swift-ci Please smoke test Linux

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.

1 participant