Skip to content

Commit

Permalink
Fix detection of ref patterns for path patterns
Browse files Browse the repository at this point in the history
I was wrong on rust-lang#19127, I thought hir-def resolver is enough for them, but it turns out not because of paths like `<Enum>::Variant` and `Type::AssocThatIsEnum::Variant`.
  • Loading branch information
ChayimFriedman2 committed Feb 17, 2025
1 parent 123f17c commit b7a9a32
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 17 deletions.
21 changes: 21 additions & 0 deletions src/tools/rust-analyzer/crates/hir-ty/src/infer/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use std::ops::{Deref, DerefMut};

use either::Either;
use hir_def::{hir::ExprOrPatId, path::Path, resolver::Resolver, type_ref::TypesMap, TypeOwnerId};
use la_arena::{Idx, RawIdx};

use crate::{
db::HirDatabase,
Expand Down Expand Up @@ -81,6 +82,26 @@ impl<'a> InferenceTyLoweringContext<'a> {
};
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path)
}

#[inline]
pub(super) fn at_path_forget_diagnostics<'b>(
&'b mut self,
path: &'b Path,
) -> PathLoweringContext<'b, 'a> {
let on_diagnostic = PathDiagnosticCallback {
data: Either::Right(PathDiagnosticCallbackData {
diagnostics: self.diagnostics,
node: ExprOrPatId::ExprId(Idx::from_raw(RawIdx::from_u32(0))),
}),
callback: |_data, _, _diag| {},
};
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path)
}

#[inline]
pub(super) fn forget_diagnostics(&mut self) {
self.ctx.diagnostics.clear();
}
}

impl<'a> Deref for InferenceTyLoweringContext<'a> {
Expand Down
13 changes: 3 additions & 10 deletions src/tools/rust-analyzer/crates/hir-ty/src/infer/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -565,16 +565,9 @@ impl InferenceContext<'_> {
| Pat::Slice { .. } => true,
Pat::Or(pats) => pats.iter().all(|p| self.is_non_ref_pat(body, *p)),
Pat::Path(path) => {
// A const is a reference pattern, but other value ns things aren't (see #16131). We don't need more than
// the hir-def resolver for this, because if there are segments left, this can only be an (associated) const.
//
// Do not use `TyLoweringContext`'s resolution, we want to ignore errors here (they'll be reported elsewhere).
let resolution = self.resolver.resolve_path_in_value_ns_fully(
self.db.upcast(),
path,
body.pat_path_hygiene(pat),
);
resolution.is_some_and(|it| !matches!(it, hir_def::resolver::ValueNs::ConstId(_)))
// A const is a reference pattern, but other value ns things aren't (see #16131).
let resolved = self.resolve_value_path_inner(path, pat.into(), true);
resolved.is_some_and(|it| !matches!(it.0, hir_def::resolver::ValueNs::ConstId(_)))
}
Pat::ConstBlock(..) => false,
Pat::Lit(expr) => !matches!(
Expand Down
26 changes: 19 additions & 7 deletions src/tools/rust-analyzer/crates/hir-ty/src/infer/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ impl InferenceContext<'_> {
}

fn resolve_value_path(&mut self, path: &Path, id: ExprOrPatId) -> Option<ValuePathResolution> {
let (value, self_subst) = self.resolve_value_path_inner(path, id)?;
let (value, self_subst) = self.resolve_value_path_inner(path, id, false)?;

let value_def: ValueTyDefId = match value {
ValueNs::FunctionId(it) => it.into(),
Expand Down Expand Up @@ -152,6 +152,7 @@ impl InferenceContext<'_> {
&mut self,
path: &Path,
id: ExprOrPatId,
no_diagnostics: bool,
) -> Option<(ValueNs, Option<chalk_ir::Substitution<Interner>>)> {
// Don't use `self.make_ty()` here as we need `orig_ns`.
let mut ctx = TyLoweringContext::new(
Expand All @@ -162,7 +163,11 @@ impl InferenceContext<'_> {
&self.diagnostics,
InferenceTyDiagnosticSource::Body,
);
let mut path_ctx = ctx.at_path(path, id);
let mut path_ctx = if no_diagnostics {
ctx.at_path_forget_diagnostics(path)
} else {
ctx.at_path(path, id)
};
let (value, self_subst) = if let Some(type_ref) = path.type_anchor() {
let last = path.segments().last()?;

Expand All @@ -172,7 +177,7 @@ impl InferenceContext<'_> {

path_ctx.ignore_last_segment();
let (ty, _) = path_ctx.lower_ty_relative_path(ty, orig_ns);
drop(ctx);
drop_ctx(ctx, no_diagnostics);
let ty = self.table.insert_type_vars(ty);
let ty = self.table.normalize_associated_types_in(ty);
self.resolve_ty_assoc_item(ty, last.name, id).map(|(it, substs)| (it, Some(substs)))?
Expand All @@ -183,7 +188,7 @@ impl InferenceContext<'_> {

match value_or_partial {
ResolveValueResult::ValueNs(it, _) => {
drop(ctx);
drop_ctx(ctx, no_diagnostics);
(it, None)
}
ResolveValueResult::Partial(def, remaining_index, _) => {
Expand All @@ -202,7 +207,7 @@ impl InferenceContext<'_> {
let self_ty = self.table.new_type_var();
let trait_ref =
path_ctx.lower_trait_ref_from_resolved_path(trait_, self_ty);
drop(ctx);
drop_ctx(ctx, no_diagnostics);
self.resolve_trait_assoc_item(trait_ref, last_segment, id)
}
(def, _) => {
Expand All @@ -212,7 +217,7 @@ impl InferenceContext<'_> {
// as Iterator>::Item::default`)
path_ctx.ignore_last_segment();
let (ty, _) = path_ctx.lower_partly_resolved_path(def, true);
drop(ctx);
drop_ctx(ctx, no_diagnostics);
if ty.is_unknown() {
return None;
}
Expand All @@ -227,7 +232,14 @@ impl InferenceContext<'_> {
}
}
};
Some((value, self_subst))
return Some((value, self_subst));

#[inline]
fn drop_ctx(mut ctx: TyLoweringContext<'_>, no_diagnostics: bool) {
if no_diagnostics {
ctx.forget_diagnostics();
}
}
}

fn add_required_obligations_for_value_path(&mut self, def: GenericDefId, subst: &Substitution) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1235,4 +1235,25 @@ fn f() {
"#,
);
}

#[test]
fn complex_enum_variant_non_ref_pat() {
check_diagnostics(
r#"
enum Enum { Variant }
trait Trait {
type Assoc;
}
impl Trait for () {
type Assoc = Enum;
}
fn foo(v: &Enum) {
let <Enum>::Variant = v;
let <() as Trait>::Assoc::Variant = v;
}
"#,
);
}
}

0 comments on commit b7a9a32

Please sign in to comment.