Swift: upgrade to 5.9 by AlexDenisov · Pull Request #14261 · github/codeql

AlexDenisov · 2023-09-19T14:04:25Z

No description provided.

redsun82 · 2023-11-07T14:43:07Z

+
+class SingleValueStmtExpr(Expr):
+    """
+    An expression that may wrap a statement which produces a single value.


"may" makes it sound like the statement would be optional. Maybe

Suggested change

An expression that may wrap a statement which produces a single value.

An expression that wraps a statement which produces a single value.

That's what the docs from compiler sources said 🤷

Hmm, then we should probably double-check whether Stmt should not be optional (I would be surprised if it could though). If not, then I would drop the may regardless of the internal compiler docs (which by the way we shouldn't really copy 😉).

geoffw0

The library changes and upgrade / downgrade scripts LGTM.

geoffw0 · 2023-11-07T16:03:00Z

I'm happy, once @redsun82 and the checks and the DCA job are all happy.

AlexDenisov · 2023-11-08T07:48:19Z

DCA looks great
I'll take care about change note for the next release (unless we need to add that we now extract one more AST node)
and the QLDoc failure can be ignored as it will be fixed after Swift: add more doc strings to generated things #14715

@geoffw0 @redsun82 anything else you'd like to see?

geoffw0 · 2023-11-08T09:21:05Z

Well I don't understand what's going on with the change note, but I remain happy with this PR. @redsun82 can approve if his concerns have been addressed.

the QLDoc failure can be ignored as it will be fixed after #14715

I think when one of these two PRs is merged, the other will need updating.

AlexDenisov · 2023-11-08T09:31:13Z

Well I don't understand what's going on with the change note, but I remain happy with this PR

AFAICT, currently we only have space for QL-related change notes. I'd like to include a note saying that we now support Swift 5.9, but previously I had to talk to a release manager to make this happen.

I think when one of these two PRs is merged, the other will need updating.

For sure, I hope this one gets in first though 😅

redsun82 · 2023-11-08T10:03:59Z

unless we need to add that we now extract one more AST node

More or less this. From the library perspective, there is a new class, so it's probably worth mentioning that. * added `SingleValueStmtExpr`. would probably be enough, I think?

AlexDenisov · 2023-11-08T10:11:38Z

unless we need to add that we now extract one more AST node

More or less this. From the library perspective, there is a new class, so it's probably worth mentioning that. * added `SingleValueStmtExpr`. would probably be enough, I think?

I'll do in the followup PR then, I don't want to restart the whole CI process for this PR. (unless there is some other changes need to happen here).

geoffw0 · 2023-11-08T11:03:21Z

AFAICT, currently we only have space for QL-related change notes. I'd like to include a note saying that we now support Swift 5.9, but previously I had to talk to a release manager to make this happen.

I feel that the library change notes are an appropriate place to describe extractor changes (i.e. swift\ql\lib\change-notes), if we do not have separate extractor change notes.

AlexDenisov · 2023-11-08T11:21:17Z

@redsun82 @geoffw0 I added change note.

geoffw0

I think this is ready to merge. Please check that @redsun82 agrees before merging.

…value-stmt-expr Swlft: fix CFG for SingleValueStmtExpr

redsun82

Thanks for the terrific work on this!

github-actions Bot added the Swift label Sep 19, 2023

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch from fec24b9 to 1bd3b91 Compare September 19, 2023 14:45

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch 2 times, most recently from 645a3d6 to 8b55cea Compare October 11, 2023 09:14

AlexDenisov mentioned this pull request Oct 11, 2023

Swift: skip declarations marked as unavailable #14437

Merged

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch 2 times, most recently from e63cd58 to f3f244c Compare October 17, 2023 12:20

AlexDenisov mentioned this pull request Oct 18, 2023

Swift: SingleValueStmtExpr migration scripts #14525

Merged

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch from 1ab4a21 to a96ac14 Compare October 31, 2023 10:24

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch from 992869e to 00235df Compare November 7, 2023 11:44

AlexDenisov added 3 commits November 7, 2023 13:40

Swift: upgrade to 5.9.1

bbde709

Swift: extract SingleValueStmtExpr

33de177

Swlft: add database upgrade scripts

2b7ce23

AlexDenisov force-pushed the alexdenisov/upgrade-to-swift-5.9 branch from 00235df to 2b7ce23 Compare November 7, 2023 12:40

AlexDenisov marked this pull request as ready for review November 7, 2023 14:22

AlexDenisov requested review from a team as code owners November 7, 2023 14:22

AlexDenisov requested review from geoffw0 and redsun82 November 7, 2023 14:24

AlexDenisov mentioned this pull request Nov 7, 2023

Swift: minimal 5.9 support #13668

Closed

redsun82 reviewed Nov 7, 2023

View reviewed changes

Comment thread swift/ql/test/extractor-tests/generated/expr/SingleValueStmtExpr/MISSING_SOURCE.txt Outdated

geoffw0 reviewed Nov 7, 2023

View reviewed changes

Comment thread swift/ql/test/library-tests/dataflow/flowsources/CONSISTENCY/CfgConsistency.expected Outdated

AlexDenisov added 3 commits November 7, 2023 17:26

Swlft: add explicit SingleValueStmtExpr test

df256e6

Swlft: fix wording in the comment

dd20a9b

Swlft: fix CFG for SingleValueStmtExpr

12f9e3a

AlexDenisov added the no-change-note-required This PR does not need a change note label Nov 8, 2023

AlexDenisov requested a review from redsun82 November 8, 2023 09:31

redsun82 reviewed Nov 8, 2023

View reviewed changes

Comment thread .../test/extractor-tests/generated/expr/SingleValueStmtExpr/CONSISTENCY/CfgConsistency.expected Outdated

Swift: add 5.9 chagenote

d510739

github-actions Bot added the documentation label Nov 8, 2023

AlexDenisov removed the no-change-note-required This PR does not need a change note label Nov 8, 2023

geoffw0 previously approved these changes Nov 8, 2023

View reviewed changes

Merge pull request #14717 from github/alexdenisov/fix-cfg-for-single-…

d8e75e6

…value-stmt-expr Swlft: fix CFG for SingleValueStmtExpr

MathiasVP dismissed geoffw0’s stale review via d8e75e6 November 8, 2023 14:26

redsun82 self-requested a review November 8, 2023 15:15

redsun82 approved these changes Nov 8, 2023

View reviewed changes

redsun82 merged commit e68826b into main Nov 8, 2023

redsun82 deleted the alexdenisov/upgrade-to-swift-5.9 branch November 8, 2023 15:15

geoffw0 mentioned this pull request Nov 9, 2023

Swift: add more doc strings to generated things #14715

Merged

	An expression that may wrap a statement which produces a single value.
	An expression that wraps a statement which produces a single value.

Uh oh!

Conversation

AlexDenisov commented Sep 19, 2023

Uh oh!

redsun82 Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

AlexDenisov Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

redsun82 Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

geoffw0 commented Nov 7, 2023

Uh oh!

AlexDenisov commented Nov 8, 2023

Uh oh!

geoffw0 commented Nov 8, 2023

Uh oh!

AlexDenisov commented Nov 8, 2023

Uh oh!

Uh oh!

redsun82 commented Nov 8, 2023

Uh oh!

AlexDenisov commented Nov 8, 2023

Uh oh!

geoffw0 commented Nov 8, 2023

Uh oh!

AlexDenisov commented Nov 8, 2023

Uh oh!

geoffw0 left a comment

Choose a reason for hiding this comment

Uh oh!

redsun82 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants