Reduce confusing `unreachable_code` lints #149044

clubby789 · 2025-11-18T00:13:28Z

Attempts to be smarter about identifying 'real'/user-written unreachable code to raise the lint

rustbot · 2025-11-18T00:13:33Z

rustbot has assigned @nnethercote.
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

cjgillot · 2025-11-18T02:00:21Z

Iirc some parts of mir building uses end_point for drop spans. Should this do the same?

Zalathar · 2025-11-18T03:26:43Z

You'll probably need to bless the coverage tests with:

./x test coverage coverage-run-rustdoc --set=build.profiler=true --bless

Coverage instrumentation unfortunately relies on a bunch of fragile heuristics that get disturbed by changes like this one, but luckily the new output seems reasonable.

clubby789 · 2025-11-18T09:49:35Z

Iirc some parts of mir building uses end_point for drop spans. Should this do the same?

Some examples of diagnostic changes with this

error: unreachable expression
  --> $DIR/void-branch.rs:10:9
   |
LL |             std::mem::uninitialized::<Void>();
   |             --------------------------------- any code following this expression is unreachable
LL |         }
   |         ^ unreachable expression

LL | #[define_opaque(WithoutLt)]
LL | fn without_lt() -> impl for<'a> Trait<'a, Assoc = WithoutLt> {}
   |                             --                                ^
   |                             |
   |                             hidden type `&'a str` captures the lifetime `'a` as defined here

I'm not sure if there's any performance implications with using sourcemap parsing on a pretty hot path, I'll perf it

rustbot · 2025-11-18T09:52:04Z

Some changes occurred in coverage tests.

cc @Zalathar

clubby789 · 2025-11-18T09:52:11Z

@bors try @rust-timer queue

Use a more accurate span for the implicit block return assignment

nnethercote

The first commit seems like a small improvement. The second commit touches a lot of test and coverage output for seemingly little gain, and doesn't seem worthwhile.

Supposedly this fixes #149042 but no test is added to cover that case. Seems like a test should be added?

View changes since this review

clubby789 · 2025-11-18T10:48:12Z

The second commit touches a lot of test and coverage output for seemingly little gain, and doesn't seem worthwhile.

Agree, I think I'll back it out (though the coverage changes were originally just not blessed with the first commit, so there's still going to be some sweeping changes there).

Seems like a test should be added?

Imo, tests/ui/uninhabited/void-branch.rs (which was added with the PR the lint change) demonstrates the span issue and fix well, do you think a dedicated test is needed?

nnethercote · 2025-11-18T11:09:56Z

do you think a dedicated test is needed?

#149042's description said a lint was issued in a case where it shouldn't be issued. void-branch.rs just sees some minor changes in output, which seems much less compelling. If you think it's good enough I will defer, it just seemed surprising.

clubby789 · 2025-11-18T11:18:34Z

Ah, this change is about turning

  |
7 |   fn check() -> () {
  |  __________________^
8 | |     make_never();
  | |     ------------ any code following this expression is unreachable
9 | | }
  | |_^ unreachable expression

into

8 |     make_never();
  |     ------------^ unreachable expression
  |     |
  |     any code following this expression is unreachable

I think there's a good argument to be made that the implicit void return should not be counted as 'code' that's unreachable. I'll see if that's feasible to reliably detect

rust-bors · 2025-11-18T12:11:16Z

☀️ Try build successful (CI)
Build commit: b2b48b3 (b2b48b3941a98a5ab0c2d3f77d25a989ed59d04f, parent: 217cb73577ed6f30a2005dd75bab01d23ec4cd60)

clubby789 · 2025-11-18T19:17:17Z

@rustbot ready

cjgillot · 2025-11-19T01:21:05Z

tests/ui/uninhabited/void-branch.rs

            std::mem::uninitialized::<Void>();
+            println!();
+            //~^ ERROR unreachable expression
        }


Where does the error point on the unchanged test? Do you mind adding another if false block with the println an keep the original one?

No error is raised within the block without the additional println (similarly to fn block in the other test I added).

Do you mind adding another if false block with the println an keep the original one?

Sorry, can you add an example? I'm not quite sure what this should look like

cjgillot · 2025-11-19T01:21:34Z

compiler/rustc_mir_build/src/builder/mod.rs

+
+            let term = bb.terminator();
+            match term.kind {
+                TerminatorKind::Goto { .. } | TerminatorKind::Return => None,


Do you mind adding a comment explaining why we return None here?

// No user code in this bb, and our goto target may be reachable via other paths
There may be other cases where we shouldn't lint and/or should label with something other than "expression" but this was sufficient for the motivating examples

I'm skeptical about the goto. What about some kind of if a { panic!() } else { unreachable!() }

Current (nightly) error
Two errors, each highlighting the if and else blocks respectively
warning: unreachable expression --> poc.rs:6:14 | 6 | if false { | ______________^ 7 | | //~^ ERROR unreachable expression 8 | | infallible(); | | ------------ any code following this expression is unreachable 9 | | } else { | |_____^ unreachable expression | warning: unreachable expression --> poc.rs:9:12 | 9 | } else { | ____________^ 10 | | //~^ ERROR unreachable expression 11 | | infallible(); | | ------------ any code following this expression is unreachable 12 | | } | |_____^ unreachable expression

With the current PR no error is emitted at all.

With the `Goto` arm removed

Two errors, both highlighting the entire if/else

warning: unreachable expression --> poc.rs:6:5 | 6 | / if false { 7 | | //~^ ERROR unreachable expression 8 | | infallible(); | | ------------ any code following this expression is unreachable 9 | | } else { 10 | | //~^ ERROR unreachable expression 11 | | infallible(); 12 | | } | |_____^ unreachable expression | warning: unreachable expression --> poc.rs:6:5 | 6 | / if false { 7 | | //~^ ERROR unreachable expression 8 | | infallible(); 9 | | } else { 10 | | //~^ ERROR unreachable expression 11 | | infallible(); | | ------------ any code following this expression is unreachable 12 | | } | |_____^ unreachable expression

r=me with a test illustrating this.

Added a test for the current behaviour.

compiler/rustc_mir_build/src/builder/mod.rs

nnethercote · 2025-11-19T08:13:33Z

This seems ok to me once @cjgillot's questions are addressed.

nnethercote · 2025-11-19T20:52:24Z

At this point I think we should just r? @cjgillot

clubby789 · 2025-11-21T21:45:08Z

@bors r=cjgillot

bors · 2025-11-21T21:45:12Z

📌 Commit 98141e0 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2025-11-22T06:28:27Z

⌛ Testing commit 98141e0 with merge af17d59...

bors · 2025-11-22T09:39:52Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing af17d59 to main...

github-actions · 2025-11-22T09:42:40Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing ea98522 (parent) -> af17d59 (this PR)

Test differences

Show 4 test diffs

Stage 1

[ui] tests/ui/uninhabited/unreachable.rs: [missing] -> pass (J1)

Stage 2

[ui] tests/ui/uninhabited/unreachable.rs: [missing] -> pass (J0)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard af17d59d71cd344fb34caab3e96c88b7c137f872 --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

aarch64-apple: 7197.0s -> 8134.8s (+13.0%)
x86_64-msvc-ext3: 5830.5s -> 6567.0s (+12.6%)
x86_64-gnu-miri: 3953.6s -> 4422.6s (+11.9%)
dist-arm-linux-gnueabi: 5333.9s -> 4896.4s (-8.2%)
dist-aarch64-msvc: 6082.5s -> 5649.5s (-7.1%)
x86_64-gnu-llvm-20-2: 6038.8s -> 5618.5s (-7.0%)
dist-x86_64-freebsd: 5007.6s -> 4661.6s (-6.9%)
dist-x86_64-illumos: 5675.5s -> 6060.0s (+6.8%)
dist-aarch64-apple: 6436.0s -> 6017.9s (-6.5%)
dist-x86_64-netbsd: 5154.1s -> 4880.0s (-5.3%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2025-11-22T11:00:44Z

Finished benchmarking commit (af17d59): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

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

Max RSS (memory usage)

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

Cycles

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

Binary size

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

Bootstrap: 472.807s -> 469.465s (-0.71%)
Artifact size: 388.29 MiB -> 388.29 MiB (0.00%)

rustbot assigned nnethercote Nov 18, 2025

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

This comment has been minimized.

Sign in to view

rust-bors bot added a commit that referenced this pull request Nov 18, 2025

Auto merge of #149044 - clubby789:implicit-return-span, r=<try>

b2b48b3

Use a more accurate span for the implicit block return assignment

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Nov 18, 2025

nnethercote reviewed Nov 18, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

nnethercote 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 Nov 18, 2025

clubby789 force-pushed the implicit-return-span branch 2 times, most recently from d1bc16c to aed6c5d Compare November 18, 2025 10:50

This comment has been minimized.

Sign in to view

This comment was marked as outdated.

Sign in to view

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Nov 18, 2025

clubby789 force-pushed the implicit-return-span branch from aed6c5d to d793f62 Compare November 18, 2025 17:40

clubby789 removed the perf-regression Performance regression. label Nov 18, 2025

clubby789 changed the title ~~Use a more accurate span for the implicit block return assignment~~ Reduce confusing unreachable_code lints Nov 18, 2025

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 Nov 18, 2025

cjgillot reviewed Nov 19, 2025

View reviewed changes

nnethercote reviewed Nov 19, 2025

View reviewed changes

compiler/rustc_mir_build/src/builder/mod.rs Outdated Show resolved Hide resolved

clubby789 force-pushed the implicit-return-span branch from d793f62 to 2d9cd22 Compare November 19, 2025 10:48

rustbot assigned cjgillot and unassigned nnethercote Nov 19, 2025

Reduce confusing unreachable_code lints

98141e0

clubby789 force-pushed the implicit-return-span branch from 2d9cd22 to 98141e0 Compare November 21, 2025 16:08

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 21, 2025

bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 22, 2025

bors merged commit af17d59 into rust-lang:main Nov 22, 2025
12 checks passed

rustbot added this to the 1.93.0 milestone Nov 22, 2025

Reduce confusing unreachable_code lints #149044

Reduce confusing unreachable_code lints #149044

Conversation

clubby789 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Nov 18, 2025

Uh oh!

This comment has been minimized.

cjgillot commented Nov 18, 2025

Uh oh!

Zalathar commented Nov 18, 2025

Uh oh!

clubby789 commented Nov 18, 2025

Uh oh!

rustbot commented Nov 18, 2025

Uh oh!

clubby789 commented Nov 18, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

nnethercote left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

clubby789 commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nnethercote commented Nov 18, 2025

Uh oh!

clubby789 commented Nov 18, 2025

Uh oh!

rust-bors bot commented Nov 18, 2025

Uh oh!

This comment has been minimized.

This comment was marked as outdated.

clubby789 commented Nov 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clubby789 Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nnethercote commented Nov 19, 2025

Uh oh!

nnethercote commented Nov 19, 2025

Uh oh!

clubby789 commented Nov 21, 2025

Uh oh!

bors commented Nov 21, 2025

Uh oh!

bors commented Nov 22, 2025

Uh oh!

bors commented Nov 22, 2025

Uh oh!

Uh oh!

github-actions bot commented Nov 22, 2025

Test differences

Stage 1

Stage 2

Job duration changes

Uh oh!

rust-timer commented Nov 22, 2025

Overall result: ✅ improvements - no action needed

Uh oh!

Reduce confusing `unreachable_code` lints #149044

Reduce confusing `unreachable_code` lints #149044

clubby789 commented Nov 18, 2025 •

edited

Loading

nnethercote left a comment •

edited by rustbot

Loading

clubby789 commented Nov 18, 2025 •

edited

Loading

clubby789 Nov 19, 2025 •

edited

Loading