Skip to content

Inherit -strict-implicit-module-context when build sub swiftinterface #66980

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

Conversation

Fushj89
Copy link
Contributor

@Fushj89 Fushj89 commented Jun 28, 2023

Cherry-pick of #67009

  • Release: Swift 5.9
  • Explanation: when run sub compiler instance to build sub swiftinterface file,we lose strict-implicit-module-context flag which causes sub invocation build clang module in the swiftinterface failed.
  • Scope: only affects the case which pass -strict-implicit-module-context to swift-frontend
  • Issue: #66743
  • Risk: Minimal, this is a relatively-new opt-in feature
  • Reviewer: @artemcm

@Fushj89 Fushj89 requested a review from a team as a code owner June 28, 2023 09:49
@Fushj89
Copy link
Contributor Author

Fushj89 commented Jun 28, 2023

@artemcm @nkcsgexi @tshortli please help review

@artemcm
Copy link
Contributor

artemcm commented Jun 28, 2023

@Fushj89 could you please first file a PR with this change targeting main?

@Fushj89
Copy link
Contributor Author

Fushj89 commented Jun 29, 2023

@artemcm I have create a new pr on main branch, see #67009.

@artemcm
Copy link
Contributor

artemcm commented Jul 5, 2023

@Fushj89 would you kindly edit the PR description to a format loosely resembling one used in e.g. #66151, outlining the scope and the risk (which here is minimal, this is a relatively-new opt-in feature).

@artemcm
Copy link
Contributor

artemcm commented Jul 5, 2023

@swift-ci test

@Fushj89
Copy link
Contributor Author

Fushj89 commented Jul 6, 2023

@artemcm I edit the PR description format. The PR is waiting for another approve, can you help find other reviewer to review it.

@Fushj89
Copy link
Contributor Author

Fushj89 commented Jul 18, 2023

@DougGregor Can you help review the PR

@artemcm artemcm requested a review from nkcsgexi July 18, 2023 15:51
@@ -1830,6 +1832,10 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
// invocation.
CompilerInvocation subInvocation = genericSubInvocation;

// save `StrictImplicitModuleContext`
bool StrictImplicitModuleContext =
subInvocation.getFrontendOptions().StrictImplicitModuleContext;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to save and restore StrictImplicitModuleContext here? It stands out from the rest of frontend options.

@artemcm
Copy link
Contributor

artemcm commented Nov 10, 2023

We are past taking changes of this nature in 5.9 for non-critical fixes, and are focusing on upcoming releases. This change made it to 5.10 so that will be the first release containing this change. Thank you @Fushj89!

@artemcm artemcm closed this Nov 10, 2023
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.

3 participants