Skip to content

[AST] Rewrite collectExportedImports() to be non-recursive. #74861

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.

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

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Jul 1, 2024

The issue with recursion here is that if there are enough modules involved, this function will blow the process stack, particularly in the case where the FileUnits are not SourceFiles, since in that instance a SmallVector gets allocated on the stack for each level of the recursion.

rdar://130527640

The issue with recursion here is that if there are enough modules
involved, this function will blow the process stack, particularly
in the case where the `FileUnit`s are not `SourceFile`s, since in
that instance a `SmallVector` gets allocated on the stack for each
level of the recursion.

rdar://130527640
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 6.0 labels Jul 1, 2024
@al45tair al45tair requested a review from a team as a code owner July 1, 2024 14:29
@al45tair
Copy link
Contributor Author

al45tair commented Jul 1, 2024

@swift-ci Please test

@al45tair
Copy link
Contributor Author

al45tair commented Jul 1, 2024

Explanation: If there are enough imports, the original code would blow the process stack. Switch to using an explicit stack on the heap instead (actually, since it's in a SmallVector, it'll be on the stack until it grows too large, so we'll still be nice and fast in the case where there are fewer imports).
Original PR: #74812
Risk: Low. It's a straightforward rewrite.
Reviewed by: @daniel-grumberg
Resolves: rdar://130527640
Tests: Looks like this code is exercised by our existing tests in various ways.

@al45tair al45tair merged commit a9cef82 into swiftlang:release/6.0 Jul 2, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants