Skip to content

Conversation

@lcnr
Copy link
Contributor

@lcnr lcnr commented Nov 7, 2025

This is really subtle ☠️ I've actually went and added testing for search_graph.ignore_candidate_head_usages to https://github.com/lcnr/search_graph_fuzz now. I should have done that when I originally implemented but didn't quite know how to do so back then.

The search graph is far too subtle to think through it manually. I've added the affected proof tree to https://github.com/rust-lang/trait-system-refactor-initiative/blob/main/notes/next-solver/search-graph/general.md#keeping-provisional-cache-entries-on-rerun. It's

  • A
    • B
      • C (depends on B and gets dropped when rerunning)
        • D (does not depend on B so we keep it around when rerunning)
          • C (irrevant candidate)
          • A
        • B
      • D
        • C (irrevant candidate)
          • D
        • A
      • rerun
      • C (use provisional cache entry which doesn't depend on B)
      • D (use provisional cache entry which doesn't depend on B)

Fixes the ICE in rust-lang/trait-system-refactor-initiative#246 (comment). I think this issue is brittle enough that adding that as a test isn't really useful. Any small change to the search graph will prevent it from testing this. We do test this fix via the fuzzer.

r? @BoxyUwU

@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 7, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Nov 7, 2025

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 7, 2025

📌 Commit c07f11a has been approved by BoxyUwU

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 7, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
don't completely reset `HeadUsages`

This is really subtle ☠️ I've actually went and added testing for `search_graph.ignore_candidate_head_usages` to https://github.com/lcnr/search_graph_fuzz now. I should have done that when I originally implemented but didn't quite know how to do so back then.

The search graph is far too subtle to think through it manually. I've added the affected proof tree to https://github.com/rust-lang/trait-system-refactor-initiative/blob/main/notes/next-solver/search-graph/general.md#keeping-provisional-cache-entries-on-rerun. It's

- A
  - B
    - C (depends on B and gets dropped when rerunning)
      - D (does not depend on B so we keep it around when rerunning)
        - C (irrevant candidate)
        - A
      - B
    - D
      - C (irrevant candidate)
        - D
      - A
    - rerun
    - C (use provisional cache entry which doesn't depend on B)
    - D (use provisional cache entry which doesn't depend on B)

Fixes the ICE in rust-lang/trait-system-refactor-initiative#246 (comment). I think this issue is brittle enough that adding that as a test isn't really useful. Any small change to the search graph will prevent it from testing this. We do test this fix via the fuzzer.

r? `@BoxyUwU`
bors added a commit that referenced this pull request Nov 8, 2025
Rollup of 15 pull requests

Successful merges:

 - #147404 (Fix issue with callsite inline attribute not being applied sometimes.)
 - #147534 (Implement SIMD funnel shifts in const-eval/Miri)
 - #147686 (update isolate_highest_one for NonZero<T>)
 - #148020 (Show backtrace on allocation failures when possible)
 - #148204 (Modify contributor email entries in .mailmap)
 - #148230 (rustdoc: Properly highlight shebang, frontmatter & weak keywords in source code pages and code blocks)
 - #148555 (Fix rust-by-example spanish translation)
 - #148556 (Fix suggestion for returning async closures)
 - #148585 ([rustdoc] Replace `print` methods with functions to improve code readability)
 - #148600 (re-use `self.get_all_attrs` result for pass indirectly attribute)
 - #148612 (Add note for identifier with attempted hygiene violation)
 - #148613 (Switch hexagon targets to rust-lld)
 - #148644 ([bootstrap] Make `--open` option work with `doc src/tools/error_index_generator`)
 - #148649 (don't completely reset `HeadUsages`)
 - #148675 (Remove eslint-js from npm dependencies)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Nov 8, 2025
don't completely reset `HeadUsages`

This is really subtle ☠️ I've actually went and added testing for `search_graph.ignore_candidate_head_usages` to https://github.com/lcnr/search_graph_fuzz now. I should have done that when I originally implemented but didn't quite know how to do so back then.

The search graph is far too subtle to think through it manually. I've added the affected proof tree to https://github.com/rust-lang/trait-system-refactor-initiative/blob/main/notes/next-solver/search-graph/general.md#keeping-provisional-cache-entries-on-rerun. It's

- A
  - B
    - C (depends on B and gets dropped when rerunning)
      - D (does not depend on B so we keep it around when rerunning)
        - C (irrevant candidate)
        - A
      - B
    - D
      - C (irrevant candidate)
        - D
      - A
    - rerun
    - C (use provisional cache entry which doesn't depend on B)
    - D (use provisional cache entry which doesn't depend on B)

Fixes the ICE in rust-lang/trait-system-refactor-initiative#246 (comment). I think this issue is brittle enough that adding that as a test isn't really useful. Any small change to the search graph will prevent it from testing this. We do test this fix via the fuzzer.

r? ``@BoxyUwU``
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.

4 participants