Skip to content

feat: Refactor path lowering and serve a new path diagnostic #19127

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

Merged
merged 4 commits into from
Feb 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions crates/hir-def/src/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,7 @@ bitflags::bitflags! {
const RUSTC_HAS_INCOHERENT_INHERENT_IMPLS = 1 << 3;
const SKIP_ARRAY_DURING_METHOD_DISPATCH = 1 << 4;
const SKIP_BOXED_SLICE_DURING_METHOD_DISPATCH = 1 << 5;
const RUSTC_PAREN_SUGAR = 1 << 6;
}
}

Expand Down Expand Up @@ -294,6 +295,9 @@ impl TraitData {
if attrs.by_key(&sym::rustc_has_incoherent_inherent_impls).exists() {
flags |= TraitFlags::RUSTC_HAS_INCOHERENT_INHERENT_IMPLS;
}
if attrs.by_key(&sym::rustc_paren_sugar).exists() {
flags |= TraitFlags::RUSTC_PAREN_SUGAR;
}

let mut skip_array_during_method_dispatch =
attrs.by_key(&sym::rustc_skip_array_during_method_dispatch).exists();
Expand Down
10 changes: 6 additions & 4 deletions crates/hir-def/src/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,7 @@ impl Path {
segments: path.mod_path().segments(),
generic_args: Some(path.generic_args()),
},
Path::LangItem(_, seg) => PathSegments {
segments: seg.as_ref().map_or(&[], |seg| std::slice::from_ref(seg)),
generic_args: None,
},
Path::LangItem(_, seg) => PathSegments { segments: seg.as_slice(), generic_args: None },
}
}

Expand Down Expand Up @@ -240,6 +237,11 @@ pub struct PathSegment<'a> {
pub args_and_bindings: Option<&'a GenericArgs>,
}

impl PathSegment<'_> {
pub const MISSING: PathSegment<'static> =
PathSegment { name: &Name::missing(), args_and_bindings: None };
}

#[derive(Debug, Clone, Copy)]
pub struct PathSegments<'a> {
segments: &'a [Name],
Expand Down
3 changes: 2 additions & 1 deletion crates/hir-def/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,8 +327,9 @@ impl Resolver {
| LangItemTarget::ImplDef(_)
| LangItemTarget::Static(_) => return None,
};
// Remaining segments start from 0 because lang paths have no segments other than the remaining.
return Some((
ResolveValueResult::Partial(type_ns, 1, None),
ResolveValueResult::Partial(type_ns, 0, None),
ResolvePathResultPrefixInfo::default(),
));
}
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-expand/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ impl Name {
/// Ideally, we want a `gensym` semantics for missing names -- each missing
/// name is equal only to itself. It's not clear how to implement this in
/// salsa though, so we punt on that bit for a moment.
pub fn missing() -> Name {
Name { symbol: sym::MISSING_NAME.clone(), ctx: () }
pub const fn missing() -> Name {
Name { symbol: sym::consts::MISSING_NAME, ctx: () }
}

/// Returns true if this is a fake name for things missing in the source code. See
Expand Down
53 changes: 19 additions & 34 deletions crates/hir-ty/src/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
pub(crate) mod cast;
pub(crate) mod closure;
mod coerce;
mod diagnostics;
pub(crate) mod diagnostics;
mod expr;
mod mutability;
mod pat;
Expand Down Expand Up @@ -1480,21 +1480,22 @@ impl<'a> InferenceContext<'a> {
&self.diagnostics,
InferenceTyDiagnosticSource::Body,
);
let mut path_ctx = ctx.at_path(path, node);
let (resolution, unresolved) = if value_ns {
let Some(res) = ctx.resolve_path_in_value_ns(path, node, HygieneId::ROOT) else {
let Some(res) = path_ctx.resolve_path_in_value_ns(HygieneId::ROOT) else {
return (self.err_ty(), None);
};
match res {
ResolveValueResult::ValueNs(value, _) => match value {
ValueNs::EnumVariantId(var) => {
let substs = ctx.substs_from_path(path, var.into(), true);
let substs = path_ctx.substs_from_path(var.into(), true);
drop(ctx);
let ty = self.db.ty(var.lookup(self.db.upcast()).parent.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
return (ty, Some(var.into()));
}
ValueNs::StructId(strukt) => {
let substs = ctx.substs_from_path(path, strukt.into(), true);
let substs = path_ctx.substs_from_path(strukt.into(), true);
drop(ctx);
let ty = self.db.ty(strukt.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
Expand All @@ -1509,7 +1510,7 @@ impl<'a> InferenceContext<'a> {
ResolveValueResult::Partial(typens, unresolved, _) => (typens, Some(unresolved)),
}
} else {
match ctx.resolve_path_in_type_ns(path, node) {
match path_ctx.resolve_path_in_type_ns() {
Some((it, idx)) => (it, idx),
None => return (self.err_ty(), None),
}
Expand All @@ -1520,21 +1521,21 @@ impl<'a> InferenceContext<'a> {
};
return match resolution {
TypeNs::AdtId(AdtId::StructId(strukt)) => {
let substs = ctx.substs_from_path(path, strukt.into(), true);
let substs = path_ctx.substs_from_path(strukt.into(), true);
drop(ctx);
let ty = self.db.ty(strukt.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
forbid_unresolved_segments((ty, Some(strukt.into())), unresolved)
}
TypeNs::AdtId(AdtId::UnionId(u)) => {
let substs = ctx.substs_from_path(path, u.into(), true);
let substs = path_ctx.substs_from_path(u.into(), true);
drop(ctx);
let ty = self.db.ty(u.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
forbid_unresolved_segments((ty, Some(u.into())), unresolved)
}
TypeNs::EnumVariantId(var) => {
let substs = ctx.substs_from_path(path, var.into(), true);
let substs = path_ctx.substs_from_path(var.into(), true);
drop(ctx);
let ty = self.db.ty(var.lookup(self.db.upcast()).parent.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
Expand All @@ -1545,31 +1546,32 @@ impl<'a> InferenceContext<'a> {
let substs = generics.placeholder_subst(self.db);
let mut ty = self.db.impl_self_ty(impl_id).substitute(Interner, &substs);

let Some(mut remaining_idx) = unresolved else {
let Some(remaining_idx) = unresolved else {
drop(ctx);
return self.resolve_variant_on_alias(ty, None, mod_path);
};

let mut remaining_segments = path.segments().skip(remaining_idx);

if remaining_segments.len() >= 2 {
path_ctx.ignore_last_segment();
}

// We need to try resolving unresolved segments one by one because each may resolve
// to a projection, which `TyLoweringContext` cannot handle on its own.
let mut tried_resolving_once = false;
while !remaining_segments.is_empty() {
let resolved_segment = path.segments().get(remaining_idx - 1).unwrap();
let current_segment = remaining_segments.take(1);

while let Some(current_segment) = remaining_segments.first() {
// If we can resolve to an enum variant, it takes priority over associated type
// of the same name.
if let Some((AdtId::EnumId(id), _)) = ty.as_adt() {
let enum_data = self.db.enum_data(id);
let name = current_segment.first().unwrap().name;
if let Some(variant) = enum_data.variant(name) {
if let Some(variant) = enum_data.variant(current_segment.name) {
return if remaining_segments.len() == 1 {
(ty, Some(variant.into()))
} else {
// We still have unresolved paths, but enum variants never have
// associated types!
// FIXME: Report an error.
(self.err_ty(), None)
};
}
Expand All @@ -1578,23 +1580,13 @@ impl<'a> InferenceContext<'a> {
if tried_resolving_once {
// FIXME: with `inherent_associated_types` this is allowed, but our `lower_partly_resolved_path()`
// will need to be updated to err at the correct segment.
//
// We need to stop here because otherwise the segment index passed to `lower_partly_resolved_path()`
// will be incorrect, and that can mess up error reporting.
break;
}

// `lower_partly_resolved_path()` returns `None` as type namespace unless
// `remaining_segments` is empty, which is never the case here. We don't know
// which namespace the new `ty` is in until normalized anyway.
(ty, _) = ctx.lower_partly_resolved_path(
node,
resolution,
resolved_segment,
current_segment,
(remaining_idx - 1) as u32,
false,
);
(ty, _) = path_ctx.lower_partly_resolved_path(resolution, false);
tried_resolving_once = true;

ty = self.table.insert_type_vars(ty);
Expand All @@ -1604,8 +1596,6 @@ impl<'a> InferenceContext<'a> {
return (self.err_ty(), None);
}

// FIXME(inherent_associated_types): update `resolution` based on `ty` here.
remaining_idx += 1;
remaining_segments = remaining_segments.skip(1);
}
drop(ctx);
Expand All @@ -1621,12 +1611,7 @@ impl<'a> InferenceContext<'a> {
(ty, variant)
}
TypeNs::TypeAliasId(it) => {
let resolved_seg = match unresolved {
None => path.segments().last().unwrap(),
Some(n) => path.segments().get(path.segments().len() - n - 1).unwrap(),
};
let substs =
ctx.substs_from_path_segment(resolved_seg, Some(it.into()), true, None);
let substs = path_ctx.substs_from_path_segment(it.into(), true, None);
drop(ctx);
let ty = self.db.ty(it.into());
let ty = self.insert_type_vars(ty.substitute(Interner, &substs));
Expand Down
76 changes: 28 additions & 48 deletions crates/hir-ty/src/infer/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,13 @@
use std::cell::RefCell;
use std::ops::{Deref, DerefMut};

use hir_def::expr_store::HygieneId;
use hir_def::hir::ExprOrPatId;
use hir_def::path::{Path, PathSegment, PathSegments};
use hir_def::resolver::{ResolveValueResult, Resolver, TypeNs};
use hir_def::type_ref::TypesMap;
use hir_def::TypeOwnerId;
use either::Either;
use hir_def::{hir::ExprOrPatId, path::Path, resolver::Resolver, type_ref::TypesMap, TypeOwnerId};

use crate::db::HirDatabase;
use crate::{
InferenceDiagnostic, InferenceTyDiagnosticSource, Ty, TyLoweringContext, TyLoweringDiagnostic,
db::HirDatabase,
lower::path::{PathDiagnosticCallback, PathLoweringContext},
InferenceDiagnostic, InferenceTyDiagnosticSource, TyLoweringContext, TyLoweringDiagnostic,
};

// Unfortunately, this struct needs to use interior mutability (but we encapsulate it)
Expand Down Expand Up @@ -44,13 +41,19 @@ impl Diagnostics {
}
}

pub(crate) struct PathDiagnosticCallbackData<'a> {
node: ExprOrPatId,
diagnostics: &'a Diagnostics,
}

pub(super) struct InferenceTyLoweringContext<'a> {
ctx: TyLoweringContext<'a>,
diagnostics: &'a Diagnostics,
source: InferenceTyDiagnosticSource,
}

impl<'a> InferenceTyLoweringContext<'a> {
#[inline]
pub(super) fn new(
db: &'a dyn HirDatabase,
resolver: &'a Resolver,
Expand All @@ -62,65 +65,42 @@ impl<'a> InferenceTyLoweringContext<'a> {
Self { ctx: TyLoweringContext::new(db, resolver, types_map, owner), diagnostics, source }
}

pub(super) fn resolve_path_in_type_ns(
&mut self,
path: &Path,
node: ExprOrPatId,
) -> Option<(TypeNs, Option<usize>)> {
let diagnostics = self.diagnostics;
self.ctx.resolve_path_in_type_ns(path, &mut |_, diag| {
diagnostics.push(InferenceDiagnostic::PathDiagnostic { node, diag })
})
}

pub(super) fn resolve_path_in_value_ns(
&mut self,
path: &Path,
node: ExprOrPatId,
hygiene_id: HygieneId,
) -> Option<ResolveValueResult> {
let diagnostics = self.diagnostics;
self.ctx.resolve_path_in_value_ns(path, hygiene_id, &mut |_, diag| {
diagnostics.push(InferenceDiagnostic::PathDiagnostic { node, diag })
})
}

pub(super) fn lower_partly_resolved_path(
&mut self,
#[inline]
pub(super) fn at_path<'b>(
&'b mut self,
path: &'b Path,
node: ExprOrPatId,
resolution: TypeNs,
resolved_segment: PathSegment<'_>,
remaining_segments: PathSegments<'_>,
resolved_segment_idx: u32,
infer_args: bool,
) -> (Ty, Option<TypeNs>) {
let diagnostics = self.diagnostics;
self.ctx.lower_partly_resolved_path(
resolution,
resolved_segment,
remaining_segments,
resolved_segment_idx,
infer_args,
&mut |_, diag| diagnostics.push(InferenceDiagnostic::PathDiagnostic { node, diag }),
)
) -> PathLoweringContext<'b, 'a> {
let on_diagnostic = PathDiagnosticCallback {
data: Either::Right(PathDiagnosticCallbackData { diagnostics: self.diagnostics, node }),
callback: |data, _, diag| {
let data = data.as_ref().right().unwrap();
data.diagnostics
.push(InferenceDiagnostic::PathDiagnostic { node: data.node, diag });
},
};
PathLoweringContext::new(&mut self.ctx, on_diagnostic, path)
}
}

impl<'a> Deref for InferenceTyLoweringContext<'a> {
type Target = TyLoweringContext<'a>;

#[inline]
fn deref(&self) -> &Self::Target {
&self.ctx
}
}

impl DerefMut for InferenceTyLoweringContext<'_> {
#[inline]
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.ctx
}
}

impl Drop for InferenceTyLoweringContext<'_> {
#[inline]
fn drop(&mut self) {
self.diagnostics
.push_ty_diagnostics(self.source, std::mem::take(&mut self.ctx.diagnostics));
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-ty/src/infer/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2132,8 +2132,8 @@ impl InferenceContext<'_> {
for kind_id in def_generics.iter_self_id().take(self_params) {
let arg = args.peek();
let arg = match (kind_id, arg) {
// Lifetimes can be elided.
// Once we have implemented lifetime elision correctly,
// Lifetimes can be inferred.
// Once we have implemented lifetime inference correctly,
// this should be handled in a proper way.
(
GenericParamId::LifetimeParamId(_),
Expand Down
14 changes: 11 additions & 3 deletions crates/hir-ty/src/infer/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,9 +564,17 @@ impl InferenceContext<'_> {
| Pat::Range { .. }
| Pat::Slice { .. } => true,
Pat::Or(pats) => pats.iter().all(|p| self.is_non_ref_pat(body, *p)),
Pat::Path(p) => {
let v = self.resolve_value_path_inner(p, pat.into());
v.is_some_and(|x| !matches!(x.0, hir_def::resolver::ValueNs::ConstId(_)))
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(_)))
}
Pat::ConstBlock(..) => false,
Pat::Lit(expr) => !matches!(
Expand Down
Loading