8000 [SE-0481] Allow weak let and make weak captures immutable by nickolas-pohilets · Pull Request #80440 · swiftlang/swift · GitHub
[go: up one dir, main page]
Skip to content

[SE-0481] Allow weak let and make weak captures immutable #80440

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

[SE-0481] Allow weak let and make weak captures immutable #80440

wants to merge 15 commits into from

Conversation

nickolas-pohilets
Copy link
Contributor
@nickolas-pohilets nickolas-pohilets commented Apr 1, 2025 •
edited
Loading

Changes were originally wrapped into experimental feature, but after proposal was approved, feature was removed and PR applies changes unconditionally.

@nickolas-pohilets nickolas-pohilets marked this pull request as draft April 1, 2025 15:54
@nickolas-pohilets nickolas-pohilets marked this pull request as ready for review April 14, 2025 14:59
@nickolas-pohilets nickolas-pohilets mentioned this pull request Apr 15, 2025
Merged
@rjmccall
Copy link
Contributor

@swift-ci Please build toolchain

@rjmccall
Copy link
Contributor
rjmccall commented May 1, 2025

@swift-ci Please build macOS toolchain

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test

@nickolas-pohilets
Fixed some tests
4aa2847 
Remaining failures:

DebugInfo/guard-let-scope4.swift
expr/closure/closures.swift
SILGen/capture-transitive.swift
SILGen/dynamic_self.swift
SILGen/unowned-class-bound-generic-parameter.swift
@nickolas-pohilets
Updated logic for diagnosting implicit self capture to handle immutab…
b138c2c 
…le weak capture (minor source-breaking change)
@nickolas-pohilets
Fixed some tests
04166e2 
Remaining failures:

SILGen/unowned-class-bound-generic-parameter.swift
@nickolas-pohilets
Updated test to reflect removal of redundant boxing of unowned refere…
3de9109 
…nces
@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please clean smoke test macOS platform

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test Linux platform

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test Windows platform

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please test macOS platform

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test macOS platform

1 similar comment
@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test macOS platform

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test Windows platform

@nickolas-pohilets
Copy link
Contributor Author

Tests fail on Windows in fails because some of the error messages differ in diagnostic spelling non-sendable vs non-Sendable:

# | C:\Users\swift-ci\jenkins\workspace\swift-PR-windows\swift\test\Concurrency\weak_ref_sendability.swift:52:26: error: incorrect message found
# |   // expected-error@+1 {{stored property 'x' of 'Sendable'-conforming class 'CheckSendability4' has non-sendable type 'NS?'}}
# |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |                          stored property 'x' of 'Sendable'-conforming class 'CheckSendability4' has non-Sendable type 'NS?'

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test

rjmccall
rjmccall requested changes May 27, 2025
View reviewed changes
Copy link
Contributor
@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

The PR title is wrong, since you're not adding an experimental feature anymore. It should probably refer to SE-0481.

@nickolas-pohilets nickolas-pohilets changed the title Added experimental feature to allow weak let and immutable weak captures [SE-0481] Allow weak let and make weak captures immutable May 28, 2025
hamishknight
hamishknight approved these changes May 29, 2025
View reviewed changes
Copy link
Contributor
@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

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

Implicit self diagnostic changes LGTM!

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test

@nickolas-pohilets
Copy link
Contributor Author

@swift-ci Please smoke test macOS platform

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hamishknight hamishknight hamishknight approved these changes

@hborla hborla Awaiting requested review from hborla hborla is a code owner

@slavapestov slavapestov Awaiting requested review from slavapestov slavapestov is a code owner

@xedin xedin Awaiting requested review from xedin xedin is a code owner

@DougGregor DougGregor Awaiting requested review from DougGregor DougGregor is a code owner

@jckarter jckarter Awaiting requested review from jckarter jckarter is a code owner

@adrian-prantl adrian-prantl Awaiting requested review from adrian-prantl adrian-prantl is a code owner

@ktoso ktoso Awaiting requested review from ktoso ktoso is a code owner

@eeckstein eeckstein Awaiting requested review from eeckstein eeckstein is a code owner

@ahoppen ahoppen Awaiting requested review from ahoppen

@bnbarham bnbarham Awaiting requested review from bnbarham bnbarham is a code owner

@CodaFi CodaFi Awaiting requested review from CodaFi CodaFi is a code owner

@rintaro rintaro Awaiting requested review from rintaro rintaro is a code owner

@rjmccall rjmccall Awaiting requested review from rjmccall

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nickolas-pohilets @rjmccall @slavapestov @hamishknight
0