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.

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

add test infra to explicitely test rustc with autodiff/enzyme disabled #131470

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

ZuseZ4
Copy link
Contributor

@ZuseZ4 ZuseZ4 commented Oct 9, 2024

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using #![feature(autodiff)]. There are two situations.

  1. We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
  2. We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a //@needs-enzyme directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:

//@ revisions: has_support no_support
//@[has_support] needs-enzyme
//@[no_support] needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}

Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? @jieyouxu

Tracking:

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Oct 9, 2024
@rustbot
Copy link
Collaborator

rustbot commented Oct 9, 2024

Some changes occurred in src/tools/compiletest

cc @jieyouxu

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, this has the right idea, but we should use the ignore- directive. See my review comment.

src/tools/compiletest/src/header/needs.rs Outdated Show resolved Hide resolved
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2024
@ZuseZ4 ZuseZ4 force-pushed the enzyme-testinfra2 branch from 748e940 to 170479b Compare October 10, 2024 08:28
@ZuseZ4 ZuseZ4 requested a review from jieyouxu October 10, 2024 08:28
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2024
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks, you can r=me aftering fixing the tidy issue

tidy error: /checkout/tests/ui/feature-gates/feature-gate-autodiff.rs: too many trailing newlines (2)

@jieyouxu
Copy link
Member

@bors delegate+ rollup

@bors
Copy link
Contributor

bors commented Oct 10, 2024

✌️ @ZuseZ4, you can now approve this pull request!

If @jieyouxu told you to "r=me" after making some further change, please make that change, then do @bors r=@jieyouxu

@ZuseZ4 ZuseZ4 force-pushed the enzyme-testinfra2 branch from 170479b to 319e517 Compare October 10, 2024 09:05
@rust-log-analyzer

This comment has been minimized.

@jieyouxu
Copy link
Member

Oh, you might need to land the test infra first, then rebase the front-end PR on to this test infra PR.

@jieyouxu jieyouxu added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2024
@ZuseZ4 ZuseZ4 force-pushed the enzyme-testinfra2 branch from 319e517 to b12dc20 Compare October 10, 2024 09:38
@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 10, 2024

yep, silly me.

@ZuseZ4
Copy link
Contributor Author

ZuseZ4 commented Oct 10, 2024

@bors r=@jieyouxu

@bors
Copy link
Contributor

bors commented Oct 10, 2024

📌 Commit b12dc20 has been approved by jieyouxu

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 10, 2024
@jieyouxu jieyouxu mentioned this pull request Oct 10, 2024
7 tasks
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 10, 2024
…ouxu

add test infra to explicitely test rustc with autodiff/enzyme disabled

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using `#![feature(autodiff)]`. There are two situations.
1) We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
2) We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a `//`@needs-enzyme`` directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:
```
//@ revisions: has_support no_support
//`@[has_support]` needs-enzyme
//`@[no_support]` needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}
```
Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? `@jieyouxu`

Tracking:

- rust-lang#124509
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 10, 2024
…ouxu

add test infra to explicitely test rustc with autodiff/enzyme disabled

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using `#![feature(autodiff)]`. There are two situations.
1) We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
2) We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a `//``@needs-enzyme``` directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:
```
//@ revisions: has_support no_support
//``@[has_support]`` needs-enzyme
//``@[no_support]`` needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}
```
Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? ``@jieyouxu``

Tracking:

- rust-lang#124509
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 10, 2024
…ouxu

add test infra to explicitely test rustc with autodiff/enzyme disabled

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using `#![feature(autodiff)]`. There are two situations.
1) We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
2) We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a `//```@needs-enzyme```` directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:
```
//@ revisions: has_support no_support
//```@[has_support]``` needs-enzyme
//```@[no_support]``` needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}
```
Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? ```@jieyouxu```

Tracking:

- rust-lang#124509
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Oct 10, 2024
…ouxu

add test infra to explicitely test rustc with autodiff/enzyme disabled

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using `#![feature(autodiff)]`. There are two situations.
1) We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
2) We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a `//````@needs-enzyme````` directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:
```
//@ revisions: has_support no_support
//````@[has_support]```` needs-enzyme
//````@[no_support]```` needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}
```
Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? ````@jieyouxu````

Tracking:

- rust-lang#124509
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#130308 (codegen_ssa: consolidate tied target checks)
 - rust-lang#130538 (Stabilize const `{slice,array}::from_mut`)
 - rust-lang#130741 (rustc_target: Add sme-b16b16 as an explicit aarch64 target feature)
 - rust-lang#131033 (Precise capturing in traits)
 - rust-lang#131442 (Match std `RUSTFLAGS` for host and target for `mir-opt` test suite to fix double std build/rebuilds)
 - rust-lang#131470 (add test infra to explicitely test rustc with autodiff/enzyme disabled)
 - rust-lang#131475 (Compiler & its UI tests: Rename remaining occurrences of "object safe" to "dyn compatible" )
 - rust-lang#131493 (Avoid redundant sysroot additions to `PATH` when linking)
 - rust-lang#131509 (Update .mailmap)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 75ccd9f into rust-lang:master Oct 10, 2024
6 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 10, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2024
Rollup merge of rust-lang#131470 - EnzymeAD:enzyme-testinfra2, r=jieyouxu

add test infra to explicitely test rustc with autodiff/enzyme disabled

I assume this is not what you want for now, but I'll update the PR once I understand how the ignore- directives work.

To summarize the situation, we want a feature gate test where we don't enable the autodiff feature using `#![feature(autodiff)]`. There are two situations.
1) We have a rustc which was build without autodiff support (current default): It gives one error about the feature being needed and one error about this rustc version being build without autodiff support.
2) We have a rustc which was build with autodiff support (i.e. for now a custom build): It gives one error about the feature being needed.

We have a `//`````@needs-enzyme`````` directive which we can use in revisions for the second case.
However, we have no way to specify that needs-enzyme implies that the second error should not be seen.
This ads a way of passing the following test:
```
//@ revisions: has_support no_support
//`````@[has_support]````` needs-enzyme
//`````@[no_support]````` needs-enzyme-disabled

#![crate_type = "lib"]

#[autodiff(dfoo, Reverse)]
//[has_support]~^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~^^ ERROR use of unstable library feature 'autodiff' [E0658]
//[no_support]~| ERROR this rustc version does not support autodiff
fn foo() {}
```
Cherry picking this PR to my frontend pr makes the test above pass in both configurations (enzyme=true/false in config.toml).
I'm open to other changes that make this testcase pass.

r? `````@jieyouxu`````

Tracking:

- rust-lang#124509
@ZuseZ4 ZuseZ4 deleted the enzyme-testinfra2 branch October 11, 2024 00:20
@jieyouxu jieyouxu added the A-compiletest Area: The compiletest test runner label Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
Development

Successfully merging this pull request may close these issues.

5 participants