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

Add support for // only-cfg-target-has-atomic to compiletest #87377

Open
jyn514 opened this issue Jul 22, 2021 · 6 comments
Open

Add support for // only-cfg-target-has-atomic to compiletest #87377

jyn514 opened this issue Jul 22, 2021 · 6 comments
Assignees
@kei519
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-feature-request Category: A feature request, i.e: not implemented / a PR. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@jyn514
Copy link
Member

jyn514 commented Jul 22, 2021 •
edited by rustbot
Loading

Right now, you have to specify the platforms and architectures you want by hand, which makes it easy to leave them out or include platforms you shouldn't. It would be nice to automate this - maybe with rustc --print cfg?

See also https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/How.20can.20I.20ignore.20tests.20on.20platforms.20without.20atomics.3F.

Originally posted by @wesleywiser in #84039 (comment)
@jyn514 jyn514 added the A-testsuite Area: The testsuite used to check the correctness of rustc label Jul 22, 2021
@inquisitivecrystal inquisitivecrystal self-assigned this Jul 22, 2021
@inquisitivecrystal inquisitivecrystal mentioned this issue Jul 23, 2021
Closed
@inquisitivecrystal inquisitivecrystal added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 24, 2021
@RalfJung
Copy link
Member

Can't you use #[cfg(target_has_atomic)] in the test to basically stub it out?

@jyn514
Copy link
Member Author

jyn514 commented Jul 24, 2021 •
edited
Loading

@RalfJung no, because that will cause the test to fail (because it's missing the errors rustc would normally emit).

@RalfJung
Copy link
Member

Ah, good point -- thanks!

@inquisitivecrystal inquisitivecrystal removed their assignment Sep 20, 2022
@jieyouxu jieyouxu added the A-compiletest Area: The compiletest test runner label May 31, 2024
@jieyouxu jieyouxu moved this from Needs Triage / Backlog to Ready in compiletest maintenance and improvements Jun 9, 2024
@jieyouxu
Copy link
Member

compiletest triage: this should probably use the needs-* machinery for capability requirements, i.e. //@ needs-atomic or a better name.

@jieyouxu jieyouxu added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Oct 17, 2024
@kei519
Copy link
Contributor

kei519 commented Nov 13, 2024

I want to try this as my first contribution. Is it ok?

@rustbot claim

kei519 added a commit to kei519/rust that referenced this issue Nov 16, 2024
@kei519
Add new test directive needs-atomic to support rust-lang#87377
abc4788
@kei519 kei519 mentioned this issue Nov 16, 2024
Closed
kei519 added a commit to kei519/rust that referenced this issue Nov 17, 2024
@kei519
Add new test directive needs-atomic to support rust-lang#87377
c0aecdc
@jieyouxu
Copy link
Member

Apparently this is a bit more tricky than I expected initially.
@rustbot label -E-easy +E-medium

@rustbot rustbot added E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Nov 20, 2024
kei519 added a commit to kei519/rust that referenced this issue Nov 27, 2024
@kei519
Add new test directive needs-target-has-atomic to support rust-lang…
a74764c 
…#87377
kei519 added a commit to kei519/rust that referenced this issue Nov 28, 2024
@kei519
Add new test directive needs-target-has-atomic to support rust-lang…
0a0db62 
…#87377
jieyouxu added a commit to jieyouxu/rust that referenced this issue Dec 2, 2024
@jieyouxu @kei519
Add needs-target-has-atomic directive
99c2322 
Before this commit, the test writer has to specify platforms and
architectures by hand for targets that have differing atomic width
support. `#[cfg(target_has_atomic)]` is not quite the same because (1)
you may have to specify additional matchers manually which has to be
maintained individually, and (2) the `#[cfg]` blocks does not
communicate to compiletest that a test would be ignored for a given
target.

This commit implements a `//@ needs-target-has-atomic` directive which
admits a comma-separated list of required atomic widths that the target
must satisfy in order for the test to run.

```
//@ needs-target-has-atomic: 8, 16, ptr
```

See <rust-lang#87377>.

Co-authored-by: kei519 <masaki.keigo.q00@kyoto-u.jp>
@jieyouxu jieyouxu mentioned this issue Dec 2, 2024
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Dec 2, 2024
@GuillaumeGomez
Rollup merge of rust-lang#133736 - jieyouxu:needs-target-has-atomic, …
80e0448 
…r=compiler-errors

Add `needs-target-has-atomic` directive

Before this PR, the test writer has to specify platforms and architectures by hand for targets that have differing atomic width support. `#[cfg(target_has_atomic="...")]` is not quite the same because (1) you may have to specify additional matchers manually which has to be maintained individually, and (2) the `#[cfg]` blocks does not communicate to compiletest that a test would be ignored for a given target.

This PR implements a `//@ needs-target-has-atomic` directive which admits a comma-separated list of required atomic widths that the target must satisfy in order for the test to run.

```
//@ needs-target-has-atomic: 8, 16, ptr
```

See <rust-lang#87377>.

This PR supersedes rust-lang#133095 and is co-authored by `@kei519,` because it was somewhat subtle, and it turned out easier to implement than to review.

rustc-dev-guide docs PR: rust-lang/rustc-dev-guide#2154
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Dec 2, 2024
@rust-timer
Unrolled build for rust-lang#133736
c09e2bd 
Rollup merge of rust-lang#133736 - jieyouxu:needs-target-has-atomic, r=compiler-errors

Add `needs-target-has-atomic` directive

Before this PR, the test writer has to specify platforms and architectures by hand for targets that have differing atomic width support. `#[cfg(target_has_atomic="...")]` is not quite the same because (1) you may have to specify additional matchers manually which has to be maintained individually, and (2) the `#[cfg]` blocks does not communicate to compiletest that a test would be ignored for a given target.

This PR implements a `//@ needs-target-has-atomic` directive which admits a comma-separated list of required atomic widths that the target must satisfy in order for the test to run.

```
//@ needs-target-has-atomic: 8, 16, ptr
```

See <rust-lang#87377>.

This PR supersedes rust-lang#133095 and is co-authored by `@kei519,` because it was somewhat subtle, and it turned out easier to implement than to review.

rustc-dev-guide docs PR: rust-lang/rustc-dev-guide#2154
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kei519 kei519

Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc C-feature-request Category: A feature request, i.e: not implemented / a PR. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
compiletest maintenance and improvements
Status: Backlog
Milestone
No milestone
6 participants
@RalfJung @inquisitivecrystal @jyn514 @jieyouxu @rustbot @kei519