Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rework interior mutability detection #12691

Merged

Conversation

Alexendoo
Copy link
Member

Replaces the existing interior mutability detection, the two main changes being

  • It now follows references/pointers e.g. struct S(&Cell)
    • mutable_key_type ignores pointers as it did before
  • The ignore_interior_mutability config now applies to types containing the ignored type, e.g. http::HeaderName

Fixes #7752
Fixes #9776
Fixes #9801

changelog: [mutable_key_type], [declare_interior_mutable_const]: now considers types that have references to interior mutable types as interior mutable

@rustbot
Copy link
Collaborator

rustbot commented Apr 18, 2024

r? @llogiq

rustbot has assigned @llogiq.
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 the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 18, 2024
@Alexendoo Alexendoo changed the title Fix ignore_interior_mutability config with indirect usages Rework interior mutability detection Apr 18, 2024
@llogiq
Copy link
Contributor

llogiq commented Apr 18, 2024

mutable_key_type ignores pointers as it did before

Isn't that a regression? There's a test case with a Result<&mut _, ()> key which appears to no longer be linted.

@Alexendoo
Copy link
Member Author

Alexendoo commented Apr 18, 2024

Forgot to mention that, the old version considered any &mut reference interior mutable but that was incorrect. The examples of &mut usize/Result<&mut usize, ()> have no interior mutability

@llogiq
Copy link
Contributor

llogiq commented Apr 19, 2024

For the mutable_key_type the problem is any mutability, not just interior mutability, so having a &mut T as a key should be linted nonetheless, as the &mut could be used to mutate the referenced value, which would violate the contract of HashMap and/or BTreeMap.

@Alexendoo
Copy link
Member Author

You can only get & access to a key in place, and you cannot mutate through a &&mut

@llogiq
Copy link
Contributor

llogiq commented Apr 20, 2024

True. OK then.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2024

📌 Commit f7aef63 has been approved by llogiq

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 20, 2024

⌛ Testing commit f7aef63 with merge 8eafeeb...

@bors
Copy link
Contributor

bors commented Apr 20, 2024

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 8eafeeb to master...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
4 participants