Skip to content

Commit

Permalink
Refactor path lowering
Browse files Browse the repository at this point in the history
And add a new diagnostic for non-`Fn` parenthesized generic args.

Path lowering started to look like a mess, with each function carrying additional parameters for the diagnostic callback (since paths can occur both in type and in expression/pattern position, and their diagnostic handling is different) and the segment index, for the diagnostics report. So I refactored it from stateless functions on `TyLoweringContext` into stateful struct, `PathLoweringContext`, that tracks the process of lowering a path from resolution til assoc types selection.
  • Loading branch information
ChayimFriedman2 committed Feb 10, 2025
1 parent 8ea6135 commit 8dafe6c
Show file tree
Hide file tree
Showing 18 changed files with 1,222 additions and 1,104 deletions.
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
2 changes: 2 additions & 0 deletions crates/hir-expand/src/name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ impl PartialEq<Name> for &Symbol {
}

impl Name {
pub const MISSING: &Name = &Name { symbol: sym::consts::missing, ctx: () };

fn new_text(text: &str) -> Name {
Name { symbol: Symbol::intern(text), ctx: () }
}
Expand Down
52 changes: 19 additions & 33 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,11 +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, 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
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

0 comments on commit 8dafe6c

Please sign in to comment.