Skip to content

Conversation

epage
Copy link
Contributor

@epage epage commented Mar 26, 2024

What does this PR try to resolve?

This intentionally borrows from RefCells before conditional code to try to prevent regressions like #13646 without exhaustively covering every case that could hit these BorrowMutErrors with complicated tests.

How should we test and review this PR?

I tested this by ensuring the registry code path panicked before rebasing on top of #13647.
Once I rebased, the panic went away.

Additional information

This intentionally borrows from `RefCell`s before conditional code
to try to prevent regressions like rust-lang#13646 without exhaustively
covering every case that could hit these `BorrowMutError`s with
complicated tests.

I tested this by ensuring the registry code path panicked before
rebasing on top of rust-lang#13647.
Once I rebased, the panic went away.
@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2024

r? @weihanglo

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

@rustbot rustbot added A-filesystem Area: issues with filesystems A-registries Area: registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2024
@epage
Copy link
Contributor Author

epage commented Mar 26, 2024

The last appveyor build is from 5 years ago and suddenly its failing on my PR?

@weihanglo
Copy link
Member

The last appveyor build is from 5 years ago and suddenly its failing on my PR?

It happened before: #11131 (comment). I then closed and created a new one.

@epage epage closed this Mar 26, 2024
@epage epage deleted the borrow_mut branch March 26, 2024 16:35
bors added a commit that referenced this pull request Mar 26, 2024
test: Add asserts to catch BorrowMutError's

### What does this PR try to resolve?

This intentionally borrows from `RefCell`s before conditional code to try to prevent regressions like #13646 without exhaustively covering every case that could hit these `BorrowMutError`s with complicated tests.

### How should we test and review this PR?

I tested this by ensuring the registry code path panicked before rebasing on top of #13647.
Once I rebased, the panic went away.

### Additional information

Split off from #13649 to try to avoid appveyor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-filesystem Area: issues with filesystems A-registries Area: registries S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants