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

make UI test OS-agnostic #133074

Merged
merged 1 commit into from
Nov 16, 2024
Merged

Conversation

japaric
Copy link
Member

@japaric japaric commented Nov 15, 2024

the internal representation of std::sync::Mutex depends on the compilation target. due to this, the compiler produces different number of errors for UI test issue-17431-6.rs depending on the compilation target.

for example, when compiling the UI test to an *-apple-* or *-qnx7* target, the "cycle detected" error is not reported

$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop

this commit replaces the problematic Mutex with UnsafeCell, which has the same internal representation regardless of the compilation target. with that change, rustc reports two errors for all compilation targets.

$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop

with this change, we can remove the ignore-apple directive as the UI test now also passes on apple targets.

the internal representation of `std::sync::Mutex` depends on the compilation target. due to this,
the compiler produces different number of errors for UI test
`issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected"
error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal
representation regardless of the compilation target. with that change, rustc reports two errors for
all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple
targets.
@rustbot
Copy link
Collaborator

rustbot commented Nov 15, 2024

r? @cjgillot

rustbot has assigned @cjgillot.
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 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 Nov 15, 2024
@Noratrieb
Copy link
Member

hey, this even makes the test a lot better and easier to understand, nice!
@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 15, 2024

📌 Commit 7f02756 has been approved by Noratrieb

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 Nov 15, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 15, 2024
…tic, r=Noratrieb

make UI test OS-agnostic

the internal representation of `std::sync::Mutex` depends on the compilation target. due to this, the compiler produces different number of errors for UI test `issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected" error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal representation regardless of the compilation target. with that change, rustc reports two errors for all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple targets.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 15, 2024
…tic, r=Noratrieb

make UI test OS-agnostic

the internal representation of `std::sync::Mutex` depends on the compilation target. due to this, the compiler produces different number of errors for UI test `issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected" error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal representation regardless of the compilation target. with that change, rustc reports two errors for all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple targets.
bors added a commit to rust-lang-ci/rust that referenced this pull request Nov 15, 2024
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#132936 (For expr `return (_ = 42);` unused_paren lint should not be triggered)
 - rust-lang#132956 (Add visit_coroutine_kind to ast::Visitor)
 - rust-lang#132978 (Mention both release *and* edition breakage for never type lints)
 - rust-lang#133074 (make UI test OS-agnostic)
 - rust-lang#133080 (Fix span edition for 2024 RPIT coming from an external macro )

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c3a632c into rust-lang:master Nov 16, 2024
6 checks passed
@rustbot rustbot added this to the 1.84.0 milestone Nov 16, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Nov 16, 2024
Rollup merge of rust-lang#133074 - ferrocene:ja-make-ui-test-os-agnostic, r=Noratrieb

make UI test OS-agnostic

the internal representation of `std::sync::Mutex` depends on the compilation target. due to this, the compiler produces different number of errors for UI test `issue-17431-6.rs` depending on the compilation target.

for example, when compiling the UI test to an `*-apple-*` or `*-qnx7*` target, the "cycle detected" error is not reported

``` console
$ cat src/lib.rs
use std::sync::Mutex;

enum Foo {
    X(Mutex<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
```

whereas rustc produces two errors for other OSes, like Linux, which is what the UI test expects

``` console
$ cargo check --target x86_64-unknown-linux-gnu 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

this commit replaces the problematic `Mutex` with `UnsafeCell`, which has the same internal representation regardless of the compilation target. with that change, rustc reports two errors for all compilation targets.

``` console
$ cat src/lib.rs
use std::cell::UnsafeCell;

enum Foo {
    X(UnsafeCell<Option<Foo>>),
}

impl Foo {
    fn bar(self) {}
}

fn main() {}

$ cargo check --target x86_64-apple-ios 2>&1 | rg '^error\['
error[E0072]: recursive type `Foo` has infinite size
error[E0391]: cycle detected when computing when `Foo` needs drop
```

with this change, we can remove the `ignore-apple` directive as the UI test now also passes on apple targets.
@japaric japaric deleted the ja-make-ui-test-os-agnostic branch November 18, 2024 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

5 participants