Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make compiletest look for exact matches in the arch table #48893

Closed
hanna-kruppe opened this issue Mar 9, 2018 · 3 comments
Closed

Make compiletest look for exact matches in the arch table #48893

hanna-kruppe opened this issue Mar 9, 2018 · 3 comments
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@hanna-kruppe
Copy link
Contributor

#47737 reported a problem with architecture names that are substrings of each other, but the problem is more general than powerpc. #48732 removed the powerpc annotations, but I argued afterwards, it would be better to improve compiletest such that it doesn't consider e.g. powerpc to be a powerpc64 target. In light of the arguments linked earlier and the additional fact that this keeps happening -- #48875 introduces the same problem for MIPS -- I really think we should revert #48732 and fix compiletest instead.

cc @nagisa @jcowgill @debris @alexcrichton
@alexcrichton
Copy link
Member

Sure! Just needs someone to do the work :)

@cuviper cuviper added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 10, 2018
@jcowgill
Copy link
Contributor

#48875 introduces the same problem for MIPS

I don't think so. The change in #48875 works because "mips64" is inserted above "mips" in the ARCH_TABLE list.

@hanna-kruppe
Copy link
Contributor Author

Hm, I think you're right. The correctness of the code depending on such a subtlety is still pretty awful, though.

@varkor varkor mentioned this issue Mar 19, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this issue Mar 21, 2018
@kennytm
Rollup merge of rust-lang#49158 - varkor:compiletest-triples, r=rkruppe
be22bab 
Make compiletest do exact matching on triples

This avoids the issues of the previous substring matching, ensuring `ARCH_TABLE` and `OS_TABLE` will no longer contain redundant entries. Fixes rust-lang#48893.

r? @rkruppe
kennytm added a commit to kennytm/rust that referenced this issue Mar 22, 2018
@kennytm
Rollup merge of rust-lang#49158 - varkor:compiletest-triples, r=rkruppe
c945725 
Make compiletest do exact matching on triples

This avoids the issues of the previous substring matching, ensuring `ARCH_TABLE` and `OS_TABLE` will no longer contain redundant entries. Fixes rust-lang#48893.

r? @rkruppe
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@cuviper @alexcrichton @jcowgill @hanna-kruppe