Skip to content

[AST] Make IsNonUserModuleRequest consider SourceFile inputs as well. #76456

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
Sep 18, 2024

Conversation

allevato
Copy link
Member

We're using a small custom frontend tool to generate indexstore data for .swiftinterface files in the SDKs. We do this by treating the .swiftinterface file as the input of an interface compilation, but this exits early because it treats it as a SourceFile instead of an external LoadedFile. This happens even if we call setIsSystemModule(true) unless we skip setting the SDK path, but that causes other problems. It seems harmless to check for SourceFiles as well, so that a tool processing an SDK interface as a direct input still gets the right state.

@allevato
Copy link
Member Author

@swift-ci please test

Copy link
Contributor

@bnbarham bnbarham left a comment

Choose a reason for hiding this comment

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

Are you able to craft a test case for this at all?

…ell.

We're using a small custom frontend tool to generate indexstore data for `.swiftinterface` files in the SDKs. We do this by treating the `.swiftinterface` file as the input of an interface compilation, but this exits early because it treats it as a `SourceFile` instead of an external `LoadedFile`. This happens even if we call `setIsSystemModule(true)` unless we skip setting the SDK path, but that causes other problems. It seems harmless to check for `SourceFile`s as well, so that a tool processing an SDK interface as a direct input still gets the right state.
@allevato
Copy link
Member Author

Are you able to craft a test case for this at all?

Added test/Index/Store/unit-system-source-file.swift, and verified that it fails when I comment out my new code path in Module.cpp.

@allevato
Copy link
Member Author

@swift-ci please smoke test

@allevato allevato requested a review from bnbarham September 17, 2024 14:10
@@ -0,0 +1,21 @@
// RUN: %empty-directory(%t)
// RUN: %empty-directory(%t/SDK)
// RUN: cp %s %t/SDK/FakeSystemModule.swift
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use split-file here instead, but... doesn't really matter I suppose.

@allevato allevato merged commit 4bb9a58 into swiftlang:main Sep 18, 2024
3 checks passed
@allevato allevato deleted the system-module-check branch September 18, 2024 23:36
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