Skip to content

Conversation

jackh726
Copy link
Member

@jackh726 jackh726 commented Oct 2, 2025

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr

@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Oct 2, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-cloud-vms rust-cloud-vms bot force-pushed the global-cache-non-concurrent-change branch from 7303769 to 0db33d1 Compare October 2, 2025 01:46
@lcnr
Copy link
Contributor

lcnr commented Oct 2, 2025

it's really useful when fuzzing as it's a case where the detection mechanism for "should I use the global cache" is broken.

you could make it a method on the search graph delegate trait and make it a noop in rustc? I do want to keep something here for fuzzing

@rust-cloud-vms rust-cloud-vms bot force-pushed the global-cache-non-concurrent-change branch from 0db33d1 to 39b36ea Compare October 2, 2025 20:54
@jackh726
Copy link
Member Author

jackh726 commented Oct 2, 2025

I changed evaluation_is_concurrent to assert_evaluation_is_concurrent and just made it a no-op.

Copy link
Contributor

@lcnr lcnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 188 to 189
// Useful for testing. If a cache entry is replaced, this should
// (in theory) only happen when concurrent.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Useful for testing. If a cache entry is replaced, this should
// (in theory) only happen when concurrent.
/// Useful for testing. If a cache entry is replaced, this should
/// (in theory) only happen when concurrent.

@jackh726 jackh726 force-pushed the global-cache-non-concurrent-change branch from 39b36ea to 55aeb17 Compare October 3, 2025 20:21
@jackh726
Copy link
Member Author

jackh726 commented Oct 3, 2025

@bors r=lcnr

@bors
Copy link
Collaborator

bors commented Oct 3, 2025

📌 Commit 55aeb17 has been approved by lcnr

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 Oct 3, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
bors added a commit that referenced this pull request Oct 4, 2025
Rollup of 14 pull requests

Successful merges:

 - #142670 (Document fully-qualified syntax in `as`' keyword doc)
 - #144908 (Fix doctest output json)
 - #145685 (add CloneFromCell and Cell::get_cloned)
 - #146330 (Bump unicode_data and printables to version 17.0.0)
 - #146451 (Fix atan2 inaccuracy in documentation)
 - #146479 (add mem::conjure_zst)
 - #147190 (std: `sys::net` cleanups)
 - #147245 (only replace the intended comma in pattern suggestions)
 - #147251 (Do not assert that a change in global cache only happens when concurrent)
 - #147269 (Add regression test for 123953)
 - #147277 (Extract common logic for iterating over features)
 - #147280 (Return to needs-llvm-components being info-only)
 - #147292 (Respect `-Z` unstable options in `rustdoc --test`)
 - #147300 (Add xtensa arch to object file creation)

r? `@ghost`
`@rustbot` modify labels: rollup
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
Zalathar added a commit to Zalathar/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
bors added a commit that referenced this pull request Oct 4, 2025
Rollup of 14 pull requests

Successful merges:

 - #142670 (Document fully-qualified syntax in `as`' keyword doc)
 - #145685 (add CloneFromCell and Cell::get_cloned)
 - #146330 (Bump unicode_data and printables to version 17.0.0)
 - #146451 (Fix atan2 inaccuracy in documentation)
 - #146479 (add mem::conjure_zst)
 - #146874 (compiler: Hint at multiple crate versions if trait impl is for wrong ADT )
 - #147117 (interpret `#[used]` as `#[used(compiler)]` on illumos)
 - #147190 (std: `sys::net` cleanups)
 - #147251 (Do not assert that a change in global cache only happens when concurrent)
 - #147280 (Return to needs-llvm-components being info-only)
 - #147288 (compiletest: Make `DirectiveLine` responsible for name/value splitting)
 - #147309 (Add documentation about unwinding to wasm targets)
 - #147315 (bless autodiff batching test)
 - #147323 (Fix top level ui tests check in tidy)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 4, 2025
…nt-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
bors added a commit that referenced this pull request Oct 4, 2025
Rollup of 11 pull requests

Successful merges:

 - #142670 (Document fully-qualified syntax in `as`' keyword doc)
 - #145685 (add CloneFromCell and Cell::get_cloned)
 - #146330 (Bump unicode_data and printables to version 17.0.0)
 - #146451 (Fix atan2 inaccuracy in documentation)
 - #146479 (add mem::conjure_zst)
 - #147117 (interpret `#[used]` as `#[used(compiler)]` on illumos)
 - #147190 (std: `sys::net` cleanups)
 - #147251 (Do not assert that a change in global cache only happens when concurrent)
 - #147280 (Return to needs-llvm-components being info-only)
 - #147288 (compiletest: Make `DirectiveLine` responsible for name/value splitting)
 - #147315 (bless autodiff batching test)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Oct 4, 2025
Rollup of 10 pull requests

Successful merges:

 - #142670 (Document fully-qualified syntax in `as`' keyword doc)
 - #145685 (add CloneFromCell and Cell::get_cloned)
 - #146330 (Bump unicode_data and printables to version 17.0.0)
 - #146451 (Fix atan2 inaccuracy in documentation)
 - #146479 (add mem::conjure_zst)
 - #147117 (interpret `#[used]` as `#[used(compiler)]` on illumos)
 - #147190 (std: `sys::net` cleanups)
 - #147251 (Do not assert that a change in global cache only happens when concurrent)
 - #147280 (Return to needs-llvm-components being info-only)
 - #147315 (bless autodiff batching test)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dade9fe into rust-lang:master Oct 4, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 4, 2025
rust-timer added a commit that referenced this pull request Oct 4, 2025
Rollup merge of #147251 - jackh726:global-cache-non-concurrent-change, r=lcnr

Do not assert that a change in global cache only happens when concurrent

Fixes rust-lang/trait-system-refactor-initiative#234

I think it should just be safe to remove this assert (rather than delaying a bug). If the previous and current result are the same, I wouldn't expect issues.

r? lcnr
@jackh726 jackh726 deleted the global-cache-non-concurrent-change branch October 4, 2025 21:52
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

search_graph incorrectly overwrites existing cache entry
5 participants