Correctly change type when adding adjustments on top of `NeverToAny` #123571

WaffleLapkin · 2024-04-06T20:58:21Z

I'm concerned that the check only caught the problem with fallback = !, because at least MIR contained <() as PartialEq>::eq(move _5, move _7) where _5: ().

I rediscovered the issue when looking at #123482's crater run.

r? compiler-errors
Fixes #120600

WaffleLapkin · 2024-04-06T20:59:55Z

tests/mir-opt/building/eq_never_type.rs

@@ -0,0 +1,12 @@
+// skip-filecheck


I tried to use // CHECK, but could make it work, because they seem to be run on the optimized MIR, at which point everything is dead-code-eliminated.

You can add a -Zmir-opt-level=0, which will avoid opts.

The call is deleted even with -Zmir-opt-level=0 :(

matthiaskrgr · 2024-04-06T21:15:56Z

tests/ui/never_type/eq-never-types.rs

@@ -0,0 +1,13 @@
+//@ check-pass
+//
+// Regression test for <https://github.com/rust-lang/rust/issues/120600>.


this could be
// issue: rust-lang/rust#120600

Is this this standardized anywhere? (this is not a normal test annotation, judging by no @...)

None of the tests currently use this formatting actually....

Is this this standardized anywhere? (this is not a normal test annotation, judging by no @...)

(we don't have any standardized issue annotations yet AFAICT)

None

🤔
git grep "// issue" | wc -l : 309

git grep "// issue.*rust-lang/rust" | wc -l : 41

afaik Jubilee planned to add a tidy lint at some point™

M. something went wrong with my search... (possibly me)

There are a lot of different styles, but sure. Changed it.

compiler-errors

Thanks @WaffleLapkin, sorry for taking so long to review :>

compiler-errors · 2024-04-18T15:28:50Z

@bors r+ rollup

bors · 2024-04-18T15:28:53Z

📌 Commit 62d956f has been approved by compiler-errors

It is now in the queue for this repository.

…, r=compiler-errors Correctly change type when adding adjustments on top of `NeverToAny` I'm concerned that the check only caught the problem with `fallback = !`, because at least MIR contained `<() as PartialEq>::eq(move _5, move _7)` where `_5: ()`. I rediscovered the issue when looking at rust-lang#123482's crater run. r? compiler-errors Fixes rust-lang#120600

…kingjubilee Rollup of 6 pull requests Successful merges: - rust-lang#117919 (Introduce perma-unstable `wasm-c-abi` flag) - rust-lang#123571 (Correctly change type when adding adjustments on top of `NeverToAny`) - rust-lang#123752 (Properly handle emojis as literal prefix in macros) - rust-lang#123980 ( Add an opt-in to store incoming edges in `VecGraph` + misc) - rust-lang#124110 (Fix negating `f16` and `f128` constants) - rust-lang#124116 (when suggesting RUST_BACKTRACE=1, add a special note for Miri's env var isolation) r? `@ghost` `@rustbot` modify labels: rollup

beetrees · 2024-04-19T00:02:30Z

I think an ICE test that was added for #120600 in #122997 4 days ago is causing this PR to fail CI. Specifically tests/crashes/120600.rs and tests/crashes/120600-2.rs.

compiler-errors · 2024-04-19T00:03:59Z

Those tests just need to be deleted then. r=me after fixing the tests.

@bors r-

(this currently produces malformed mir: we call `eq` with first argument not being a reference)

WaffleLapkin · 2024-04-19T11:36:23Z

@bors r=compiler-errors

bors · 2024-04-19T11:36:26Z

📌 Commit e5a6d8d has been approved by compiler-errors

It is now in the queue for this repository.

…, r=compiler-errors Correctly change type when adding adjustments on top of `NeverToAny` I'm concerned that the check only caught the problem with `fallback = !`, because at least MIR contained `<() as PartialEq>::eq(move _5, move _7)` where `_5: ()`. I rediscovered the issue when looking at rust-lang#123482's crater run. r? compiler-errors Fixes rust-lang#120600

…iaskrgr Rollup of 4 pull requests Successful merges: - rust-lang#123571 (Correctly change type when adding adjustments on top of `NeverToAny`) - rust-lang#123729 (run-make: refactor out command wrappers for `clang` and `llvm-readobj`) - rust-lang#124149 (rustdoc-search: fix description on aliases in results) - rust-lang#124155 (bootstrap: don't use rayon for sysinfo) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 5 pull requests Successful merges: - rust-lang#123571 (Correctly change type when adding adjustments on top of `NeverToAny`) - rust-lang#123729 (run-make: refactor out command wrappers for `clang` and `llvm-readobj`) - rust-lang#124106 (Don't repeatedly duplicate TAIT lifetimes for each subsequently nested TAIT) - rust-lang#124149 (rustdoc-search: fix description on aliases in results) - rust-lang#124155 (bootstrap: don't use rayon for sysinfo) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#123571 - WaffleLapkin:properly-adjust-never, r=compiler-errors Correctly change type when adding adjustments on top of `NeverToAny` I'm concerned that the check only caught the problem with `fallback = !`, because at least MIR contained `<() as PartialEq>::eq(move _5, move _7)` where `_5: ()`. I rediscovered the issue when looking at rust-lang#123482's crater run. r? compiler-errors Fixes rust-lang#120600

rustbot assigned compiler-errors Apr 6, 2024

WaffleLapkin changed the title ~~Correctly change type when adding adjustments on top of NeverToAny~~ Correctly change type when adding adjustments on top of NeverToAny Apr 6, 2024

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 Apr 6, 2024

WaffleLapkin commented Apr 6, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

matthiaskrgr reviewed Apr 6, 2024

View reviewed changes

rust-cloud-vms bot force-pushed the properly-adjust-never branch 2 times, most recently from 86ea8a9 to 62d956f Compare April 6, 2024 21:56

WaffleLapkin mentioned this pull request Apr 11, 2024

Tracking Issue for Rust 2024: Make ! fall back to ! #123748

Closed

8 tasks

compiler-errors approved these changes Apr 18, 2024

View reviewed changes

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 18, 2024

GuillaumeGomez mentioned this pull request Apr 18, 2024

Rollup of 10 pull requests #124131

Closed

GuillaumeGomez mentioned this pull request Apr 18, 2024

Rollup of 7 pull requests #124133

Closed

workingjubilee mentioned this pull request Apr 18, 2024

Rollup of 6 pull requests #124140

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 19, 2024

WaffleLapkin added 4 commits April 19, 2024 11:05

Add regression test for issue 120600

662d276

Add a test for a == b where a: !, b: !

4d749ca

(this currently produces malformed mir: we call `eq` with first argument not being a reference)

Correctly change type when adding adjustments on top of NeverToAny

0bbe362

Remove old ICE tests that no longer ICE (yay!)

e5a6d8d

rust-cloud-vms bot force-pushed the properly-adjust-never branch from 62d956f to e5a6d8d Compare April 19, 2024 11:35

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 Apr 19, 2024

matthiaskrgr mentioned this pull request Apr 19, 2024

Rollup of 4 pull requests #124161

Closed

matthiaskrgr mentioned this pull request Apr 19, 2024

Rollup of 5 pull requests #124170

Merged

bors merged commit 6c4657c into rust-lang:master Apr 19, 2024
12 checks passed

rustbot added this to the 1.79.0 milestone Apr 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correctly change type when adding adjustments on top of `NeverToAny` #123571

Correctly change type when adding adjustments on top of `NeverToAny` #123571

WaffleLapkin commented Apr 6, 2024

WaffleLapkin Apr 6, 2024

cjgillot Apr 6, 2024

WaffleLapkin Apr 6, 2024

This comment has been minimized.

matthiaskrgr Apr 6, 2024

WaffleLapkin Apr 6, 2024

WaffleLapkin Apr 6, 2024

jieyouxu Apr 6, 2024

matthiaskrgr Apr 6, 2024

WaffleLapkin Apr 6, 2024

compiler-errors left a comment

compiler-errors commented Apr 18, 2024

bors commented Apr 18, 2024

beetrees commented Apr 19, 2024

compiler-errors commented Apr 19, 2024

WaffleLapkin commented Apr 19, 2024

bors commented Apr 19, 2024

Correctly change type when adding adjustments on top of NeverToAny #123571

Correctly change type when adding adjustments on top of NeverToAny #123571

Conversation

WaffleLapkin commented Apr 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Apr 18, 2024

bors commented Apr 18, 2024

beetrees commented Apr 19, 2024

compiler-errors commented Apr 19, 2024

WaffleLapkin commented Apr 19, 2024

bors commented Apr 19, 2024

Correctly change type when adding adjustments on top of `NeverToAny` #123571

Correctly change type when adding adjustments on top of `NeverToAny` #123571