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

fix: Goto definition for deref_mut #16696

Merged
merged 3 commits into from Feb 27, 2024

Conversation

ShoyuVanilla
Copy link
Contributor

Fixes #16520

pub(crate) fn resolve_prefix_expr(
&self,
db: &dyn HirDatabase,
prefix_expr: &ast::PrefixExpr,
) -> Option<FunctionId> {
let (lang_item, fn_name) = match prefix_expr.op_kind()? {
ast::UnaryOp::Deref => (LangItem::Deref, name![deref]),
ast::UnaryOp::Not => (LangItem::Not, name![not]),
ast::UnaryOp::Neg => (LangItem::Neg, name![neg]),
};
let ty = self.ty_of_expr(db, &prefix_expr.expr()?)?;
let (op_trait, op_fn) = self.lang_trait_fn(db, lang_item, &fn_name)?;
// HACK: subst for all methods coincides with that for their trait because the methods
// don't have any generic parameters, so we skip building another subst for the methods.
let substs = hir_ty::TyBuilder::subst_for_def(db, op_trait, None).push(ty.clone()).build();
Some(self.resolve_impl_method_or_trait_def(db, op_fn, substs))
}

As we can see from the above, current implementation routes all dereferencing prefix operations to Deref::deref implementation, not regarding mutabilities.

Expr::UnaryOp { expr, op: UnaryOp::Deref } => {
if let Some((f, _)) = self.result.method_resolutions.get_mut(&tgt_expr) {
if mutability == Mutability::Mut {
if let Some(deref_trait) = self
.db
.lang_item(self.table.trait_env.krate, LangItem::DerefMut)
.and_then(|l| l.as_trait())
{
if let Some(deref_fn) =
self.db.trait_data(deref_trait).method_by_name(&name![deref_mut])
{
*f = deref_fn;
}
}
}
}
self.infer_mut_expr(*expr, mutability);
}

Since we are resolving them already in mutability inferences, we can use those results for proper deref / deref_mut routing.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 27, 2024
@Veykril
Copy link
Member

Veykril commented Feb 27, 2024

Thanks! I wonder if we record that for indexing expressions as well so that we can resolve IndexMut in a similar fashion.
@bors r+

@bors
Copy link
Collaborator

bors commented Feb 27, 2024

📌 Commit 6124431 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Feb 27, 2024

⌛ Testing commit 6124431 with merge a41cec9...

@ShoyuVanilla
Copy link
Contributor Author

ShoyuVanilla commented Feb 27, 2024

I wonder if we record that for indexing expressions as well so that we can resolve IndexMut in a similar fashion.

Good point. I'll check for that case tomorrow

@bors
Copy link
Collaborator

bors commented Feb 27, 2024

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing a41cec9 to master...

@bors bors merged commit a41cec9 into rust-lang:master Feb 27, 2024
11 checks passed
@ShoyuVanilla ShoyuVanilla deleted the fix-goto-deref-mut branch February 27, 2024 16:13
bors added a commit that referenced this pull request Feb 29, 2024
fix: Goto definition for `index_mut`

Mostly same with #16696.
https://github.com/rust-lang/rust-analyzer/blob/0ac05c05271f31c43d31017cbd288e8737a0edb0/crates/hir-ty/src/infer/mutability.rs#L103-L133
Thankfully, we are doing similar method resolutions so we can use them like the mentioned PR.
As there are only three `LangItem`s having `Mut` in there names; `FnMut`, `DerefMut` and `IndexMut`, I think that this is the last one 😄
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
Development

Successfully merging this pull request may close these issues.

Misdirect the definition of deref_mut to deref
4 participants