Skip to content

trait_selection: Preserve eager normalization failures - #1

Closed
Dnreikronos wants to merge 1 commit into
trait_solver/resolve_vars_after_deep_normalizefrom
trait_solver_preserve_normalization_failure
Closed

Dnreikronos wants to merge 1 commit into
trait_solver/resolve_vars_after_deep_normalizefrom
trait_solver_preserve_normalization_failure

Conversation

@Dnreikronos

@Dnreikronos Dnreikronos commented Sep 10, 2026 •

Copy link
Copy Markdown
Owner

Draft experiment for rust-lang#161407, based on its branch so the diff shows the alternative discussed on Zulip.

The nested inherent associated type case fails during eager normalization, but the fallback can accept it. Eager normalization reduces the inner alias to &'b (), then tries to equate Foo<for<'b> fn(&'b ())> with the impl's Foo<fn(&'a ())>. The impl lifetime sits outside the for<'b> binder, so that relation fails the leak check. The fallback retries the original nested aliases. That turns the lifetime equalities into constraints returned by nested goals, which instantiate_and_apply_query_response marks VisibleForLeakCheck::No. The parent leak check misses them and the fallback succeeds. Resolving the resulting inference variable at the end hides the rejected relation.

I changed normalize_with_universes to return a Result carrying the failed obligation. Deep normalization returns that error immediately, and I removed the final resolve_vars_if_possible workaround. The general policy for nested constraints stays as it is.

For the infallible normalize entry point, I kept recovery inside the function for now. It catches the error, puts the failed obligation in the list, then runs ReplaceAliasWithInfer over the original value. The visitor still creates inference variables and obligations for aliases without escaping bound vars. For aliases with escaping bound vars, it leaves the alias in place and no longer creates placeholder obligations. Keeping the original failure means a successful fallback can't silently discard it.

My thinking was to get deep normalization to report the failure while keeping the existing normalize callers working. I think making normalization fallible throughout is the better direction, with HIR typeck reporting the failure and replacing aliases with TyKind::Error. I haven't made those caller changes here. That's the part I'd like to settle before taking this further.

@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Dnreikronos pushed a commit that referenced this pull request Oct 10, 2026
Speed up tidy again

Tidy has slowed down quite a bit over the years (rust-lang#81833, rust-lang#105829), so let's recover some perf

```
OLD

$ hyperfine -w 1 "./x test tidy"
Benchmark #1: ./x test tidy
  Time (mean ± σ):      5.076 s ±  0.057 s    [User: 12.724 s, System: 6.707 s]
  Range (min … max):    4.972 s …  5.148 s    10 runs

$ taskset -c 0-5 hyperfine -w 1 "./x test tidy"
Benchmark #1: ./x test tidy
  Time (mean ± σ):      7.973 s ±  0.099 s    [User: 12.072 s, System: 5.550 s]
  Range (min … max):    7.838 s …  8.184 s    10 runs

NEW

$ hyperfine -w 1 "./x test tidy"
Benchmark #1: ./x test tidy
  Time (mean ± σ):      3.134 s ±  0.055 s    [User: 7.993 s, System: 6.842 s]
  Range (min … max):    3.047 s …  3.204 s    10 runs

$ taskset -c 0-5 hyperfine -w 1 "./x test tidy"
Benchmark #1: ./x test tidy
  Time (mean ± σ):      3.921 s ±  0.129 s    [User: 7.204 s, System: 5.563 s]
  Range (min … max):    3.740 s …  4.069 s    10 runs
```

After this about half of the remaining time is burned by bootstrap doing git and cargo stuff.

<img width="2546" height="1061" alt="image" src="https://github.com/user-attachments/assets/0049a525-9c2e-433a-a88c-94e8063dce5b" />

Fixes: rust-lang#141074

----

LLM disclosure: I used copilot's autocomplete. Most suggestions were trivial (or discarded for being incorrect), with the exception of the Semaphore type which it produced almost wholesale. I compared it to implementations in the wild and they look almost identical, so I hope this too falls under "trivial".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant