Skip to content

Fix LifetimeDependenceDiagnostics to ignore closure captures #72635

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 7 commits into from
Mar 28, 2024

Conversation

atrick
Copy link
Contributor

@atrick atrick commented Mar 27, 2024

Fix LifetimeDependenceDiagnostics to ignore closure captures

ClosureLifetimeFixup now emits mark_dependence [nonescaping]. Those should be
ignored by diagnostics. In the capture case, the dependence has already been
resolved, and may not match the SIL patterns that we expect for source-level
lifetime dependencies.

Fixes rdar://125375685 ([nonescapable] Fix lifetime-dependence diagnostics in the stdlib)x

This allows us to always-enable lifetime-depenence diagnostics.

This PR also includes obvious, harmless fixes to move-only diagnostics to allow adding
nonescapable unit tests.

Fix MoveOnlyDiagnostics, ConsumOperator...Checkers diagnostics

Emitting a note with an invalid source location is actively
harmful. It confuses users and tools, makes it impossible to write
unit tests. In this case, the note simply says "use here", so it's
completely free of information without the source location.

atrick added 7 commits March 27, 2024 09:13
This fixes bugs when ~Escapable types depended on values that are passed to 'consume'.

The consume operator diagnostics are broken when dependent values are
present. This sidesteps the problem for lifetime dependence. And we
generally want to diagnose lifetime dependence after all move-only
related diagnostics. That way, using a dependent value after consume
provides a more informative diagnostic about the dependent value and
its scope.
Emitting a note with an invalid source location is actively
harmful. It confuses users and tools, makes it impossible to write
unit tests. In this case, the note simply says "use here", so it's
completely free of information without the source location.
ClosureLifetimeFixup now emits mark_dependence [nonescaping]. Those should be
ignored by diagnostics. In the capture case, the dependence has already been
resolved, and may not match the SIL patterns that we expect for source-level
lifetime dependencies.

Fixes rdar://125375685 ([nonescapable] Fix lifetime-dependence diagnostics in the stdlib)
-enable-experimental-feature NonescapableTypes now only controls syntax and some type inferrence features.
@atrick atrick requested review from kavon and eeckstein as code owners March 27, 2024 20:59
@atrick atrick requested review from meg-gupta and gottesmm and removed request for kavon and eeckstein March 27, 2024 20:59
@atrick
Copy link
Contributor Author

atrick commented Mar 27, 2024

@swift-ci test

@atrick
Copy link
Contributor Author

atrick commented Mar 27, 2024

@swift-ci benchmark

@atrick
Copy link
Contributor Author

atrick commented Mar 27, 2024

@swift-ci test source compatibility

Copy link
Contributor

@meg-gupta meg-gupta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

//
// FIXME: why is DeinitDevirtualizer in the middle of the mandatory pipeline,
// and what passes/compilation modes depend on it? This pass is never executed
// or tested without '-Xllvm enable-deinit-devirtualizer'.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eeckstein if you know why the DeinitDevirtualizer runs in the middle of the mandatory pipeline or know of any passes that depend on it, in embedded mode or otherwise, please document that in the pass header. I couldn't figure it out after wasting an hour investigating.

@atrick
Copy link
Contributor Author

atrick commented Mar 27, 2024

@swift-ci Please Build Toolchain macOS Platform

@atrick
Copy link
Contributor Author

atrick commented Mar 28, 2024

source compat Siesta appears to be failing on main also

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.

2 participants