Skip to content

Do some preparation work for compiletest check-cfg #123577

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Apr 8, 2024

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Apr 6, 2024

This PR does several preparation work for having always-on check-cfg in compiletest.

In particular, this PR does two main things:

  • It unifies all the always-false cfgs under the FALSE cfg (as it seems to be the convention under tests/ui)
  • It also removes some useless conditions

This is done ahead of the introduction of the always-on check-cfg in compiletest to reduce the amount of changes in that follow-up work. I also think that this is useful even without that follow-up work.

Urgau added 3 commits April 7, 2024 01:16
Since they are never set and don't have impact on the test.

Or for the cfg-panic tests are already tested with check-cfg.
While `false` is accepted by `--cfg` it isn't by `#[cfg(false)]`
since in that context `false` is the boolean not a ident.
@rustbot
Copy link
Collaborator

rustbot commented Apr 6, 2024

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 6, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 6, 2024

Some changes occurred in tests/ui/sanitizer

cc @rust-lang/project-exploit-mitigations, @rcvalle

@Noratrieb
Copy link
Member

The cfg(FALSE) could be replaced with cfg(any()), but I agree that cfg(FALSE) is easier to understand and should be preferred.

@Urgau
Copy link
Member Author

Urgau commented Apr 7, 2024

Yeah, I also though of using cfg(any()) but decided against since FALSE is already used quite extensively in our UI test suite and as you mentioned it is much easier to understand.

@BoxyUwU
Copy link
Member

BoxyUwU commented Apr 8, 2024

r? @oli-obk

@rustbot rustbot assigned oli-obk and unassigned BoxyUwU Apr 8, 2024
@oli-obk
Copy link
Contributor

oli-obk commented Apr 8, 2024

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 8, 2024

📌 Commit 47ff773 has been approved by oli-obk

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-review Status: Awaiting review from the assignee but also interested parties. labels Apr 8, 2024
@bors
Copy link
Collaborator

bors commented Apr 8, 2024

⌛ Testing commit 47ff773 with merge 0e5f520...

@bors
Copy link
Collaborator

bors commented Apr 8, 2024

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 0e5f520 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 8, 2024
@bors bors merged commit 0e5f520 into rust-lang:master Apr 8, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 8, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0e5f520): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.2% [1.2%, 1.2%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 667.477s -> 666.9s (-0.09%)
Artifact size: 318.52 MiB -> 318.48 MiB (-0.01%)

@Urgau Urgau deleted the prep-work-for-compiletest-check-cfg branch April 9, 2024 21:42
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Apr 10, 2024
…eck-cfg-2, r=jieyouxu

Further cleanup cfgs in the UI test suite

This PR does more cleanup of cfgs in our UI test suite, in preparation for adding automatic always on check-cfg (but is IMO worth landing even without that follow up).

To be more specific this PR:
 - replaces (the last remaining) never true cfgs by the `FALSE` cfg
 - fix `proc-macro/derive-helper-configured.rs` *(typo in directive)*
 - and comment some current unused `#[cfg_attr]` *(missing revisions)*

Follow-up to rust-lang#123577.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2024
Rollup merge of rust-lang#123702 - Urgau:prep-work-for-compiletest-check-cfg-2, r=jieyouxu

Further cleanup cfgs in the UI test suite

This PR does more cleanup of cfgs in our UI test suite, in preparation for adding automatic always on check-cfg (but is IMO worth landing even without that follow up).

To be more specific this PR:
 - replaces (the last remaining) never true cfgs by the `FALSE` cfg
 - fix `proc-macro/derive-helper-configured.rs` *(typo in directive)*
 - and comment some current unused `#[cfg_attr]` *(missing revisions)*

Follow-up to rust-lang#123577.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 25, 2024
…youxu

Enable `--check-cfg` by default in UI tests

This PR enables-by-default `--check-cfg` in UI tests, now that it has become stable.

To do so this PR does 2 main things:
 - it introduce the `no-auto-check-cfg` directive to `compiletest`, to prevent any `--check-cfg` args (only to be used for `--check-cfg` tests)
 - it updates the _remaining_[^1] UI tests by either:
     - allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
     - give the appropriate check-cfg args
     - or expect the lint, when it useful

[^1]: some preparation work was done in rust-lang#123577 rust-lang#123702

I highly recommend reviewing this PR commit-by-commit.

r? `@jieyouxu`
fmease added a commit to fmease/rust that referenced this pull request May 2, 2024
…youxu

Enable `--check-cfg` by default in UI tests

This PR enables-by-default `--check-cfg` in UI tests, now that it has become stable.

To do so this PR does 2 main things:
 - it introduce the `no-auto-check-cfg` directive to `compiletest`, to prevent any `--check-cfg` args (only to be used for `--check-cfg` tests)
 - it updates the _remaining_[^1] UI tests by either:
     - allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
     - give the appropriate check-cfg args
     - or expect the lint, when it useful

[^1]: some preparation work was done in rust-lang#123577 rust-lang#123702

I highly recommend reviewing this PR commit-by-commit.

r? `@jieyouxu`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 2, 2024
…youxu

Enable `--check-cfg` by default in UI tests

This PR enables-by-default `--check-cfg` in UI tests, now that it has become stable.

To do so this PR does 2 main things:
 - it introduce the `no-auto-check-cfg` directive to `compiletest`, to prevent any `--check-cfg` args (only to be used for `--check-cfg` tests)
 - it updates the _remaining_[^1] UI tests by either:
     - allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
     - give the appropriate check-cfg args
     - or expect the lint, when it useful

[^1]: some preparation work was done in rust-lang#123577 rust-lang#123702

I highly recommend reviewing this PR commit-by-commit.

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2024
Enable `--check-cfg` by default in UI tests

This PR enables-by-default `--check-cfg` in UI tests, now that it has become stable.

To do so this PR does 2 main things:
 - it introduce the `no-auto-check-cfg` directive to `compiletest`, to prevent any `--check-cfg` args (only to be used for `--check-cfg` tests)
 - it updates the _remaining_[^1] UI tests by either:
     - allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
     - give the appropriate check-cfg args
     - or expect the lint, when it useful

[^1]: some preparation work was done in rust-lang#123577 rust-lang#123702

I highly recommend reviewing this PR commit-by-commit.

r? `@jieyouxu`
bors added a commit to rust-lang-ci/rust that referenced this pull request May 4, 2024
Enable `--check-cfg` by default in UI tests

This PR enables-by-default `--check-cfg` in UI tests, now that it has become stable.

To do so this PR does 2 main things:
 - it introduce the `no-auto-check-cfg` directive to `compiletest`, to prevent any `--check-cfg` args (only to be used for `--check-cfg` tests)
 - it updates the _remaining_[^1] UI tests by either:
     - allowing the lint when neither expecting the lint nor giving the check-cfg args make sense
     - give the appropriate check-cfg args
     - or expect the lint, when it useful

[^1]: some preparation work was done in rust-lang#123577 rust-lang#123702

I highly recommend reviewing this PR commit-by-commit.

r? `@jieyouxu`
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
merged-by-bors This PR was explicitly merged by bors. PG-exploit-mitigations Project group: Exploit mitigations S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants