-
Notifications
You must be signed in to change notification settings - Fork 341
[clang][Darwin] Simplify deployment version assignment in the Driver (#142013) #10802
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
base: swift/release/6.2
Are you sure you want to change the base?
[clang][Darwin] Simplify deployment version assignment in the Driver (#142013) #10802
Conversation
…lvm#142013) To be able to handle all of the ways the platform & deployment version can be represented in command line flags, the Darwin toolchain holds a type `DarwinPlatform` to help represent them. This patch simplifies the logic by: * reducing the amount of work done between string & version tuples conversions * renaming variables to reduce confusion about what target triple information is being manipulated. * allowing implicit transformation of macOS10.16 -> 11, there are other places in the compiler where this happens, and it was a bit confusing that the driver didn't do that for the cc1 call. This is not a major refactor, but more simple & common tweaks across the file, in hopes to make it more readable. (cherry picked from commit 752adc3)
@swfit-ci please test |
@swift-ci please test |
@swift-ci please test macOS |
@swift-ci please smoke test macOS |
@swift-ci please test macOS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// We allow multiple ways to set or default the OS | ||
// version used for compilation. When set, UnderlyingOSVersion represents | ||
// the intended version to match the platform information computed from | ||
// arguments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: seems like this can be reflowed.
// We allow multiple ways to set or default the OS | |
// version used for compilation. When set, UnderlyingOSVersion represents | |
// the intended version to match the platform information computed from | |
// arguments. | |
// We allow multiple ways to set or default the OS version used for | |
// compilation. When set, UnderlyingOSVersion represents the intended version | |
// to match the platform information computed from arguments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only change is formatting, right? clang-format
may be the reason for this, but if not, I'll fix it upstream & cherry-pick.
To be able to handle all of the ways the platform & deployment version can be represented in command line flags, the Darwin toolchain holds a type
DarwinPlatform
to help represent them. This patch simplifies the logic by:This is not a major refactor, but more simple & common tweaks across the file, in hopes to make it more readable.
(cherry picked from commit 752adc3)