[SE-0408] Enable Pack Iteration #67594
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simanerush
force-pushed
the
simanerush/pack-iteration-impl
branch
2 times, most recently
from
4b1b8c5 to
1fcb8da
Compare
simanerush
changed the title
simanerush
force-pushed
the
simanerush/pack-iteration-impl
branch
from
1fcb8da to
42a4340
Compare
simanerush
commented
Aug 10, 2023
simanerush
commented
Aug 10, 2023
simanerush
changed the title
xedin
reviewed
Aug 18, 2023
simanerush
requested review from
adrian-prantl,
AnthonyLatsis,
artemcm,
hamishknight,
ahoppen,
bnbarham,
CodaFi,
DougGregor,
rintaro,
zoecarver,
hyp,
egorzhdan,
MaxDesiatov,
etcwilde,
ktoso and
kavon
as code owners
opened element generic environments containing same-element requirements.
simanerush
force-pushed
the
simanerush/pack-iteration-impl
branch
from
bf57556 to
48cb330
Compare
2 tasks
xedin
approved these changes
Dec 5, 2023
|
@swift-ci Please smoke test |
|
@swift-ci please test source compatibility |
|
Are we fine with merging that last joint commit with this PR? Why did we need it here in the first place? |
|
Yes please do! It's a latent bug that manifested in one of Sima's test cases IIRC. The bug fix isn't really relevant to the PR I originally included it in (the same-element requirement PR) other than I also hit the issue in one of the test cases there. |
Comment on lines
+1258
to
+1261
| JumpDest loopDest = createJumpDest(S->getBody()); | ||
|
|
||
| // Set the destinations for 'break' and 'continue'. | ||
| JumpDest endDest = createJumpDest(S->getBody()); |
There was a problem hiding this comment.
I think loopDest and endDest would be far clearer as continueDest and breakDest for anyone yet to read this. Just an opinion.
There was a problem hiding this comment.
I just saw this. I think this makes a lot of sense, thank you!
hborla
approved these changes
Dec 8, 2023
There was a problem hiding this comment.
This is in fantastic shape, thank you for your work on this feature! The review discussion above is very focused in on details, targeted bug fixes (e.g. adding a diagnosis for where clauses during constraint generation), improvements to documentation, etc, and this patch is already quite large, so I suggest landing this as is and addressing further feedback in subsequent PRs. This will help make more incremental progress, and it'll make the subsequent changes easier to review.
|
This pull broke the community Android CI, maybe because a host Swift compiler isn't available there and so |
|
Thanks @finagolfin, I reproduced the issue locally and I'm investigating it. Here's the symbolicated stack trace: SwiftCompilerSources currently use bootstrapping on macOS CI, so I'm not sure what the difference is, but I don't think it's related to |
Just to clarify, macOS CI uses bootstrap-with-hostlibs vs Android which is using full bootstrap, so there could possibly be a difference there. |
|
I filed #70328 to track this issue |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the implementation of this SE-0408.
What's working
Supported
for casewhererepeat)Design
TaggedUnionfor representingForEachStmtInfo, where it can either beSequenceIterationInfoor the newPackIterationInfo.CSGen/CSApplyfor handling the pack iteration case.SILGenStmt: it handles constructing the loop from a pack expansion expression