Skip to content

Windows triple normalization #80

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 4 commits into from
Mar 5, 2020

Conversation

sjavora
Copy link
Contributor

@sjavora sjavora commented Mar 1, 2020

Adds logic from llvm::Triple::normalize, which enables all the asserts from testNormalizeWindows as well as some other edge cases.

Notably, objectFormat for some Windows triples still doesn't parse. As far as I can tell, this is because Triple.EnvInfo checks for both environment and object format in the fourth position, so in strings like i686-pc-windows-msvc-elf, the elf is just totally ignored.

parsedEnv.substring.starts(with: "androideabi") {
let androidVersion = parsedEnv.substring.dropFirst("androideabi".count)

parser.components[3] = "android\(androidVersion)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no test for this - I didn't find one in TripleTest.cpp and I don't really know what a "valid" triple like this would look like.

@DougGregor
Copy link
Member

@swift-ci please test

Copy link
Contributor

@aciidgh aciidgh left a comment

Choose a reason for hiding this comment

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

LGTM, I just left some minor nitpicks.

@sjavora sjavora force-pushed the triple-normalization-upgrade branch from e4219c9 to 3d096a4 Compare March 5, 2020 18:36
@sjavora sjavora requested a review from aciidgh March 5, 2020 18:36
@sjavora
Copy link
Contributor Author

sjavora commented Mar 5, 2020

Updated.

@aciidgh
Copy link
Contributor

aciidgh commented Mar 5, 2020

@swift-ci test

1 similar comment
@DougGregor
Copy link
Member

@swift-ci test

@DougGregor DougGregor merged commit 983422b into swiftlang:master Mar 5, 2020
@sjavora sjavora deleted the triple-normalization-upgrade branch March 5, 2020 20:11
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