Skip to content

[cherry-pick 5.5] Task locals revisions #37158

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 14 commits into from
Apr 30, 2021
Merged

[cherry-pick 5.5] Task locals revisions #37158

merged 14 commits into from
Apr 30, 2021

Conversation

ktoso
Copy link
Contributor

@ktoso ktoso commented Apr 30, 2021 •
edited
Loading

Approved by @DougGregor et al for 5.5 cherry pick even though has minor ABI change.

Cherry pick of: #36959

@ktoso ktoso requested review from rjmccall and DougGregor April 30, 2021 04:13
@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

@swift-ci please smoke test

@ktoso ktoso added the r5.5 label Apr 30, 2021
@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

@swift-ci please test

@ktoso
[TaskLocals] Windows: fix missing io.h
f5b3d85 
TaskLocals print a verbose error before crashing if API is abused within a task-group.
This printing needs `io.h` on windows.
@ktoso ktoso added the concurrency Feature: umbrella label for concurrency language features label Apr 30, 2021
@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

@swift-ci please test

1 similar comment
@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - f5b3d85

@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

Very weird:

<unknown>:0: error: module file '/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-release/5.5/buildbot_linux/swiftpm-linux-x86_64/x86_64-unknown-linux-gnu/swift-driver/module-cache/PQH7SIAO640I/CDispatch-RAPLRX6YU1H6.pcm' is out of date and needs to be rebuilt: signature mismatch
01:46:42.656 <unknown>:0: note: imported by module '_SwiftDispatchOverlayShims' in '/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-release/5.5/buildbot_linux/swiftpm-linux-x86_64/x86_64-unknown-linux-gnu/swift-driver/module-cache/PQH7SIAO640I/_SwiftDispatchOverlayShims-93BFK8SDU5SN.pcm'
01:46:42.656 <unknown>:0: error: missing required module '_SwiftDispatchOverlayShims'
01:46:42.656 ninja: build stopped: subcommand failed.

@ktoso
Copy link
Contributor Author

ktoso commented Apr 30, 2021

@swift-ci please test Linux

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - f5b3d85

kavon
kavon approved these changes Apr 30, 2021
View reviewed changes
Copy link
Member

@kavon kavon left a comment

Choose a reason for hiding this comment

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

Mostly had feedback on things that are not a blocker for this cherry-pick, but would be good to address in main in the future.

lib/Sema/TypeCheckPropertyWrapper.cpp Outdated
Comment on lines 262 to 267
paramTy->dump();
paramTy->getCanonicalType()->dump();
neverTy->dump();
neverTy->getCanonicalType()->dump();
param->dump();
param->getType()->dump();
Copy link
Member

Choose a reason for hiding this comment

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

We should remove these debugging dumps.

Copy link
Member

Choose a reason for hiding this comment

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

Whoops, yeah. These need to be removed. Thanks, Kavon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops damn sorry, will cleanup on main too then

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok doug already fixed, thanks

include/swift/AST/DiagnosticsSema.def
Comment on lines +5610 to +5611
"property %0, must be static because property wrapper %1 can only "
"be applied to static properties",
Copy link
Member

Choose a reason for hiding this comment

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

Might be simpler to just say: "property wrapper ___ can only be applied to static properties"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I wanted to include the name of the property, you sure we'd prefer the shorter message?

Copy link
Member

Choose a reason for hiding this comment

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

I think the name of the property isn't needed, because the error message will already be pointing to the var decl with the problematic wrapper on it, whether in the terminal or xcode.

lib/Sema/TypeCheckPropertyWrapper.cpp
Comment on lines +414 to +420
// if (requireNoEnclosingInstance) { // && !valueVar->isStatic()) {
// // this means that the property wrapper must be declared on a static property
// valueVar->diagnose(
// diag::property_wrapper_var_must_be_static, valueVar->getName());
// return PropertyWrapperTypeInfo();
// result
// }
Copy link
Member

Choose a reason for hiding this comment

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

Why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, will remove -- this is a leftover, it was the wrong place to do the diagnosis after all so moved elsewhere

@kavon kavon self-requested a review April 30, 2021 15:41
kavon
kavon requested changes Apr 30, 2021
View reviewed changes
@DougGregor
Copy link
Member

@swift-ci please test macOS

DougGregor
DougGregor reviewed Apr 30, 2021
View reviewed changes
@DougGregor
Copy link
Member

@swift-ci please test

@kavon kavon self-requested a review April 30, 2021 16:26
@DougGregor DougGregor merged commit e4498b2 into swiftlang:release/5.5 Apr 30, 2021
@ktoso ktoso deleted the pick-tasklocals branch May 1, 2021 01:06
@AnthonyLatsis AnthonyLatsis added 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5 labels Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kavon kavon kavon approved these changes

@DougGregor DougGregor DougGregor approved these changes

@rjmccall rjmccall Awaiting requested review from rjmccall

Assignees
No one assigned
Labels
concurrency Feature: umbrella label for concurrency language features 🍒 release cherry pick Flag: Release branch cherry picks swift 5.5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ktoso @swift-ci @DougGregor @kavon @AnthonyLatsis