Skip to content

Sink SwiftHost.cpp into HostInfo (NFC) #5165

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
Aug 23, 2022

Conversation

adrian-prantl
Copy link

This refactors the code that currently lives in Expression/SwiftHost
to find a new home in HostInfo. The idea is to eventually allow
different implementations per host platform. Note that this is
intentionally just moving the code around, there has been no attempt
at cleaning up the code or interface.

rdar://92640609

@adrian-prantl
Copy link
Author

@swift-ci test

Copy link
Member

@compnerd compnerd left a comment

Choose a reason for hiding this comment

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

How does this hook up the HostInfoPosix into the Windows path? It feels like this regresses the Windows path.

This refactors the code that currently lives in Expression/SwiftHost
to find a new home in HostInfo. The idea is to eventually allow
different implementations per host platform. Note that this is
intentionally just moving the code around, there has been no attempt
at cleaning up the code or interface.

rdar://92640609
@adrian-prantl
Copy link
Author

How does this hook up the HostInfoPosix into the Windows path? It feels like this regresses the Windows path.

Thanks @compnerd, you are right, HostInfoWindows does not inherit from HostInfoPosix. So I made the patch even more complex, by moving the shared functionality into common/ :-)

@adrian-prantl
Copy link
Author

@swift-ci test

@adrian-prantl adrian-prantl merged commit 75cfd24 into swiftlang:stable/20220421 Aug 23, 2022
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