Skip to content

Conversation

artemcm
Copy link
Contributor

@artemcm artemcm commented Sep 22, 2025

This can lead to bizarre interactions where the source module under scan is queried as a dependency of itself.

On top of that, properly restrict embedded compilation mode to load serialized binary modules, only.

Resolves rdar://159706486

@artemcm
Copy link
Contributor Author

artemcm commented Sep 22, 2025

@swift-ci smoke test

@artemcm artemcm marked this pull request as ready for review September 22, 2025 21:21
Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

Do we have a test case for the overlay/underlying module change?

return;
}

// Avoid overlay lookup for the underlying module of a known Swift module
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest reword this comment and commit message a bit. Overlay/underlying is a generic term but here we only skip the current module and its underlying (@_exported import) module. I didn't quite get what this does in the first place when I read the comment and message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we have a test case for the overlay/underlying module change?

It's very tricky to write one, as even when it was queried we already had logic in place to ensure we don't add the result to the graph. e.g. When scanning A, we would query Swift overlay for underlying Clang module A and find some other A.swiftmodule in the search paths, we would simply discard this result. Now we don't even query it but it's challenging to verify this behavior change.

@artemcm artemcm force-pushed the RestrictEmbeddedToBinarySwift branch from 221b96d to 34a314d Compare September 22, 2025 21:48
@artemcm
Copy link
Contributor Author

artemcm commented Sep 22, 2025

@swift-ci smoke test

@artemcm artemcm enabled auto-merge September 22, 2025 22:04
@artemcm
Copy link
Contributor Author

artemcm commented Sep 22, 2025

@swift-ci smoke test Linux platform

@artemcm
Copy link
Contributor Author

artemcm commented Sep 23, 2025

@swift-ci smoke test macOS platform

@artemcm
Copy link
Contributor Author

artemcm commented Sep 23, 2025

@swift-ci smoke test

Avoid Swift Overlay lookup for the underlying clang module of a known Swift module.
i.e. When computing set of Swift Overlay dependencies for module 'A', which depends on a Clang module 'A', ensure we don't lookup Swift module 'A' itself here - this can lead to bizarre interactions where the source module under scan is queried as a dependency of itself.

Resolves rdar://159706486
@artemcm artemcm force-pushed the RestrictEmbeddedToBinarySwift branch from 34a314d to e9a7519 Compare September 23, 2025 17:06
@artemcm
Copy link
Contributor Author

artemcm commented Sep 23, 2025

@swift-ci smoke test

@artemcm artemcm merged commit 391a18d into swiftlang:main Sep 23, 2025
3 checks passed
@artemcm artemcm deleted the RestrictEmbeddedToBinarySwift branch September 23, 2025 22:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants