Skip to content

[6.1] Add version checks & fix pointer auth for nonisolated deinit #78107

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
Merged
merged 13 commits into from
Jan 12, 2025
Merged

[6.1] Add version checks & fix pointer auth for nonisolated deinit #78107

merged 13 commits into from
Jan 12, 2025

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Dec 11, 2024 •
edited
Loading

Description: Correct the version guards for the new isolated deinit runtime feature and fix missing pointer auth in the feature.
Scope/Impact: Low, only the new isolated deinit feature is impacted.
Risk: Medium, this brings back isolated deinit logic and fixes pointer auth for it. Code changes are limited to the new feature though.
Testing: CI testing and verified using extra arm64e pre-merge testing to check the pointer auth fix
Reviewed by: @rjmccall?

Original PR: #77730
Radar: rdar://95142593

@ktoso ktoso requested a review from a team as a code owner December 11, 2024 08:57
@ktoso ktoso requested review from rjmccall and hborla December 11, 2024 08:57
@ktoso
Copy link
Contributor Author

ktoso commented Dec 11, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Dec 11, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Dec 12, 2024

@swift-ci please test

1 similar comment
@ktoso
Copy link
Contributor Author

ktoso commented Dec 12, 2024

@swift-ci please test

@ktoso ktoso added concurrency Feature: umbrella label for concurrency language features swift 6.1 labels Dec 12, 2024
@ktoso
Copy link
Contributor Author

ktoso commented Dec 12, 2024

@swift-ci please test

@ktoso ktoso force-pushed the pick-mpokhylets-isolated-deinit-version branch from a1826e0 to 55b8e80 Compare December 13, 2024 01:51
@ktoso
Copy link
Contributor Author

ktoso commented Dec 13, 2024

@swift-ci please test

hborla
hborla requested changes Dec 13, 2024
View reviewed changes
@ktoso ktoso force-pushed the pick-mpokhylets-isolated-deinit-version branch from fbc6303 to 8e61e8a Compare December 13, 2024 23:51
@ktoso
Copy link
Contributor Author

ktoso commented Dec 13, 2024

@swift-ci please test

@ktoso ktoso force-pushed the pick-mpokhylets-isolated-deinit-version branch from 8e61e8a to db34ec1 Compare December 16, 2024 01:43
@ktoso ktoso force-pushed the pick-mpokhylets-isolated-deinit-version branch from db34ec1 to 87191a2 Compare December 16, 2024 03:05
@ktoso
Copy link
Contributor Author

ktoso commented Dec 16, 2024

@swift-ci please test

hborla
hborla reviewed Dec 18, 2024
View reviewed changes
include/swift/AST/FeatureAvailability.def Outdated
@@ -76,7 +76,6 @@ FEATURE(InitRawStructMetadata, (6, 0))

FEATURE(LayoutStringValueWitnesses, (6, 1))
FEATURE(CreateTaskWithConsumedFunction, (6, 1))
FEATURE(IsolatedDeinit, (6, 1))
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this for checking availability if the experimental feature is enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I see, yeah let me bring that back.

lib/Sema/TypeCheckAttr.cpp Outdated
if (auto nominal = dyn_cast<NominalTypeDecl>(D->getDeclContext())) {
if (!isa<ClassDecl>(nominal)) {
// only classes and actors can have isolated deinit.
diagnoseAndRemoveAttr(attr, diag::isolated_deinit_on_value_type);
return;
}
}

TypeChecker::checkAvailability(
Copy link
Member

Choose a reason for hiding this comment

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

Similar comment to above: should this be kept around for if the experimental feature is enabled?

@ktoso
Copy link
Contributor Author

ktoso commented Dec 18, 2024

I think I've gotten this right now then: 0046c75 (#78107)

@ktoso
Copy link
Contributor Author

ktoso commented Dec 18, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Dec 20, 2024 •
edited
Loading

Thanks for approving. CI Jobs disappeared heh. Running again.

@ktoso
Copy link
Contributor Author

ktoso commented Dec 20, 2024

@swift-ci please test

@ktoso
Copy link
Contributor Author

ktoso commented Jan 4, 2025

Testing again as it didn’t make it before branches got locked

@ktoso
Copy link
Contributor Author

ktoso commented Jan 4, 2025

@swift-ci please test

@ktoso ktoso merged commit 7b4a1ab into swiftlang:release/6.1 Jan 12, 2025
5 checks passed
@ktoso ktoso deleted the pick-mpokhylets-isolated-deinit-version branch January 12, 2025 06:42
@DougGregor DougGregor changed the title [6.1] Make isolated deinit non-experiment and add version checks & fix pointer auth [6.1] Add version checks & fix pointer auth for nonisolated deinit Jan 13, 2025
@ktoso ktoso mentioned this pull request Mar 21, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hborla hborla hborla approved these changes

@rjmccall rjmccall Awaiting requested review from rjmccall

Assignees
No one assigned
Labels
concurrency Feature: umbrella label for concurrency language features swift 6.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ktoso @hborla @nickolas-pohilets