Unify handling of path diagnostics in hir-ty

Because it was a mess.

Previously, pretty much you had to handle all path diagnostics manually: remember to check for them and handle them. Now, we wrap the resolver in `TyLoweringContext` and ensure proper error reporting.

This means that you don't have to worry about them: most of the things are handled automatically, and things that cannot will create a compile-time error (forcing you top `drop(ty_lowering_context);`) if forgotten, instead of silently dropping the diagnostics.

The real place for error reporting is in the hir-def resolver, because there are other things resolving, both in hir-ty and in hir-def, and they all need to ensure proper diagnostics. But this is a good start, and future compatible.

This commit also ensures proper path diagnostics for value/pattern paths, which is why it's marked "feat".
This commit is contained in:
Chayim Refael Friedman 2024-12-22 18:07:27 +02:00
parent 82896b2cc4
commit cc11e1a796
14 changed files with 848 additions and 247 deletions

View file

@ -85,6 +85,8 @@ use crate::{
FxIndexMap, LocalModuleId, Lookup, MacroExpander, MacroId, ModuleId, ProcMacroId, UseId,
};
pub use self::path_resolution::ResolvePathResultPrefixInfo;
const PREDEFINED_TOOLS: &[SmolStr] = &[
SmolStr::new_static("clippy"),
SmolStr::new_static("rustfmt"),
@ -615,13 +617,15 @@ impl DefMap {
(res.resolved_def, res.segment_index)
}
/// The first `Option<usize>` points at the `Enum` segment in case of `Enum::Variant`, the second
/// points at the unresolved segments.
pub(crate) fn resolve_path_locally(
&self,
db: &dyn DefDatabase,
original_module: LocalModuleId,
path: &ModPath,
shadow: BuiltinShadowMode,
) -> (PerNs, Option<usize>) {
) -> (PerNs, Option<usize>, ResolvePathResultPrefixInfo) {
let res = self.resolve_path_fp_with_macro_single(
db,
ResolveMode::Other,
@ -630,7 +634,7 @@ impl DefMap {
shadow,
None, // Currently this function isn't used for macro resolution.
);
(res.resolved_def, res.segment_index)
(res.resolved_def, res.segment_index, res.prefix_info)
}
/// Ascends the `DefMap` hierarchy and calls `f` with every `DefMap` and containing module.

View file

@ -38,7 +38,7 @@ use crate::{
attr_resolution::{attr_macro_as_call_id, derive_macro_as_call_id},
diagnostics::DefDiagnostic,
mod_resolution::ModDir,
path_resolution::ReachedFixedPoint,
path_resolution::{ReachedFixedPoint, ResolvePathResultPrefixInfo},
proc_macro::{parse_macro_name_and_helper_attrs, ProcMacroDef, ProcMacroKind},
sub_namespace_match, BuiltinShadowMode, DefMap, MacroSubNs, ModuleData, ModuleOrigin,
ResolveMode,
@ -797,7 +797,7 @@ impl DefCollector<'_> {
return PartialResolvedImport::Unresolved;
}
if res.from_differing_crate {
if let ResolvePathResultPrefixInfo::DifferingCrate = res.prefix_info {
return PartialResolvedImport::Resolved(
def.filter_visibility(|v| matches!(v, Visibility::Public)),
);

View file

@ -43,21 +43,34 @@ pub(super) struct ResolvePathResult {
pub(super) resolved_def: PerNs,
pub(super) segment_index: Option<usize>,
pub(super) reached_fixedpoint: ReachedFixedPoint,
pub(super) from_differing_crate: bool,
pub(super) prefix_info: ResolvePathResultPrefixInfo,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum ResolvePathResultPrefixInfo {
None,
DifferingCrate,
/// Path of the form `Enum::Variant` (and not `Variant` alone).
Enum,
}
impl ResolvePathResult {
fn empty(reached_fixedpoint: ReachedFixedPoint) -> ResolvePathResult {
ResolvePathResult::new(PerNs::none(), reached_fixedpoint, None, false)
ResolvePathResult::new(
PerNs::none(),
reached_fixedpoint,
None,
ResolvePathResultPrefixInfo::None,
)
}
fn new(
resolved_def: PerNs,
reached_fixedpoint: ReachedFixedPoint,
segment_index: Option<usize>,
from_differing_crate: bool,
prefix_info: ResolvePathResultPrefixInfo,
) -> ResolvePathResult {
ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, from_differing_crate }
ResolvePathResult { resolved_def, segment_index, reached_fixedpoint, prefix_info }
}
}
@ -157,7 +170,17 @@ impl DefMap {
if result.reached_fixedpoint == ReachedFixedPoint::No {
result.reached_fixedpoint = new.reached_fixedpoint;
}
result.from_differing_crate |= new.from_differing_crate;
result.prefix_info = match (result.prefix_info, new.prefix_info) {
(ResolvePathResultPrefixInfo::None, it) => it,
(ResolvePathResultPrefixInfo::DifferingCrate, _) => {
ResolvePathResultPrefixInfo::DifferingCrate
}
(
ResolvePathResultPrefixInfo::Enum,
ResolvePathResultPrefixInfo::DifferingCrate,
) => ResolvePathResultPrefixInfo::DifferingCrate,
(ResolvePathResultPrefixInfo::Enum, _) => ResolvePathResultPrefixInfo::Enum,
};
result.segment_index = match (result.segment_index, new.segment_index) {
(Some(idx), None) => Some(idx),
(Some(old), Some(new)) => Some(old.max(new)),
@ -403,14 +426,14 @@ impl DefMap {
fn resolve_remaining_segments<'a>(
&self,
segments: impl Iterator<Item = (usize, &'a Name)>,
mut segments: impl Iterator<Item = (usize, &'a Name)>,
mut curr_per_ns: PerNs,
path: &ModPath,
db: &dyn DefDatabase,
shadow: BuiltinShadowMode,
original_module: LocalModuleId,
) -> ResolvePathResult {
for (i, segment) in segments {
while let Some((i, segment)) = segments.next() {
let curr = match curr_per_ns.take_types_full() {
Some(r) => r,
None => {
@ -443,7 +466,7 @@ impl DefMap {
def,
ReachedFixedPoint::Yes,
s.map(|s| s + i),
true,
ResolvePathResultPrefixInfo::DifferingCrate,
);
}
@ -488,17 +511,28 @@ impl DefMap {
),
})
});
match res {
Some(res) => res,
None => {
return ResolvePathResult::new(
PerNs::types(e.into(), curr.vis, curr.import),
ReachedFixedPoint::Yes,
Some(i),
false,
)
// FIXME: Need to filter visibility here and below? Not sure.
return match res {
Some(res) => {
if segments.next().is_some() {
// Enum variants are in value namespace, segments left => no resolution.
ResolvePathResult::empty(ReachedFixedPoint::No)
} else {
ResolvePathResult::new(
res,
ReachedFixedPoint::Yes,
None,
ResolvePathResultPrefixInfo::Enum,
)
}
}
}
None => ResolvePathResult::new(
PerNs::types(e.into(), curr.vis, curr.import),
ReachedFixedPoint::Yes,
Some(i),
ResolvePathResultPrefixInfo::None,
),
};
}
s => {
// could be an inherent method call in UFCS form
@ -513,7 +547,7 @@ impl DefMap {
PerNs::types(s, curr.vis, curr.import),
ReachedFixedPoint::Yes,
Some(i),
false,
ResolvePathResultPrefixInfo::None,
);
}
};
@ -522,7 +556,12 @@ impl DefMap {
.filter_visibility(|vis| vis.is_visible_from_def_map(db, self, original_module));
}
ResolvePathResult::new(curr_per_ns, ReachedFixedPoint::Yes, None, false)
ResolvePathResult::new(
curr_per_ns,
ReachedFixedPoint::Yes,
None,
ResolvePathResultPrefixInfo::None,
)
}
fn resolve_name_in_module(

View file

@ -240,6 +240,7 @@ pub struct PathSegment<'a> {
pub args_and_bindings: Option<&'a GenericArgs>,
}
#[derive(Debug, Clone, Copy)]
pub struct PathSegments<'a> {
segments: &'a [Name],
generic_args: Option<&'a [Option<GenericArgs>]>,
@ -259,6 +260,7 @@ impl<'a> PathSegments<'a> {
pub fn last(&self) -> Option<PathSegment<'a>> {
self.get(self.len().checked_sub(1)?)
}
pub fn get(&self, idx: usize) -> Option<PathSegment<'a>> {
let res = PathSegment {
name: self.segments.get(idx)?,
@ -266,24 +268,37 @@ impl<'a> PathSegments<'a> {
};
Some(res)
}
pub fn skip(&self, len: usize) -> PathSegments<'a> {
PathSegments {
segments: self.segments.get(len..).unwrap_or(&[]),
generic_args: self.generic_args.and_then(|it| it.get(len..)),
}
}
pub fn take(&self, len: usize) -> PathSegments<'a> {
PathSegments {
segments: self.segments.get(..len).unwrap_or(self.segments),
generic_args: self.generic_args.map(|it| it.get(..len).unwrap_or(it)),
}
}
pub fn strip_last(&self) -> PathSegments<'a> {
PathSegments {
segments: self.segments.split_last().map_or(&[], |it| it.1),
generic_args: self.generic_args.map(|it| it.split_last().map_or(&[][..], |it| it.1)),
}
}
pub fn strip_last_two(&self) -> PathSegments<'a> {
PathSegments {
segments: self.segments.get(..self.segments.len().saturating_sub(2)).unwrap_or(&[]),
generic_args: self
.generic_args
.map(|it| it.get(..it.len().saturating_sub(2)).unwrap_or(&[])),
}
}
pub fn iter(&self) -> impl Iterator<Item = PathSegment<'a>> {
self.segments
.iter()

View file

@ -21,7 +21,7 @@ use crate::{
hir::{BindingId, ExprId, LabelId},
item_scope::{BuiltinShadowMode, ImportId, ImportOrExternCrate, BUILTIN_SCOPE},
lang_item::LangItemTarget,
nameres::{DefMap, MacroSubNs},
nameres::{DefMap, MacroSubNs, ResolvePathResultPrefixInfo},
path::{ModPath, Path, PathKind},
per_ns::PerNs,
type_ref::{LifetimeRef, TypesMap},
@ -263,25 +263,37 @@ impl Resolver {
&self,
db: &dyn DefDatabase,
path: &Path,
mut hygiene_id: HygieneId,
hygiene_id: HygieneId,
) -> Option<ResolveValueResult> {
self.resolve_path_in_value_ns_with_prefix_info(db, path, hygiene_id).map(|(it, _)| it)
}
pub fn resolve_path_in_value_ns_with_prefix_info(
&self,
db: &dyn DefDatabase,
path: &Path,
mut hygiene_id: HygieneId,
) -> Option<(ResolveValueResult, ResolvePathResultPrefixInfo)> {
let path = match path {
Path::BarePath(mod_path) => mod_path,
Path::Normal(it) => it.mod_path(),
Path::LangItem(l, None) => {
return Some(ResolveValueResult::ValueNs(
match *l {
LangItemTarget::Function(it) => ValueNs::FunctionId(it),
LangItemTarget::Static(it) => ValueNs::StaticId(it),
LangItemTarget::Struct(it) => ValueNs::StructId(it),
LangItemTarget::EnumVariant(it) => ValueNs::EnumVariantId(it),
LangItemTarget::Union(_)
| LangItemTarget::ImplDef(_)
| LangItemTarget::TypeAlias(_)
| LangItemTarget::Trait(_)
| LangItemTarget::EnumId(_) => return None,
},
None,
return Some((
ResolveValueResult::ValueNs(
match *l {
LangItemTarget::Function(it) => ValueNs::FunctionId(it),
LangItemTarget::Static(it) => ValueNs::StaticId(it),
LangItemTarget::Struct(it) => ValueNs::StructId(it),
LangItemTarget::EnumVariant(it) => ValueNs::EnumVariantId(it),
LangItemTarget::Union(_)
| LangItemTarget::ImplDef(_)
| LangItemTarget::TypeAlias(_)
| LangItemTarget::Trait(_)
| LangItemTarget::EnumId(_) => return None,
},
None,
),
ResolvePathResultPrefixInfo::None,
))
}
Path::LangItem(l, Some(_)) => {
@ -296,7 +308,10 @@ impl Resolver {
| LangItemTarget::ImplDef(_)
| LangItemTarget::Static(_) => return None,
};
return Some(ResolveValueResult::Partial(type_ns, 1, None));
return Some((
ResolveValueResult::Partial(type_ns, 1, None),
ResolvePathResultPrefixInfo::None,
));
}
};
let n_segments = path.segments().len();
@ -326,9 +341,12 @@ impl Resolver {
});
if let Some(e) = entry {
return Some(ResolveValueResult::ValueNs(
ValueNs::LocalBinding(e.binding()),
None,
return Some((
ResolveValueResult::ValueNs(
ValueNs::LocalBinding(e.binding()),
None,
),
ResolvePathResultPrefixInfo::None,
));
}
}
@ -350,14 +368,17 @@ impl Resolver {
Scope::GenericParams { params, def } => {
if let Some(id) = params.find_const_by_name(first_name, *def) {
let val = ValueNs::GenericParam(id);
return Some(ResolveValueResult::ValueNs(val, None));
return Some((
ResolveValueResult::ValueNs(val, None),
ResolvePathResultPrefixInfo::None,
));
}
}
&Scope::ImplDefScope(impl_) => {
if *first_name == sym::Self_.clone() {
return Some(ResolveValueResult::ValueNs(
ValueNs::ImplSelf(impl_),
None,
return Some((
ResolveValueResult::ValueNs(ValueNs::ImplSelf(impl_), None),
ResolvePathResultPrefixInfo::None,
));
}
}
@ -377,22 +398,27 @@ impl Resolver {
Scope::GenericParams { params, def } => {
if let Some(id) = params.find_type_by_name(first_name, *def) {
let ty = TypeNs::GenericParam(id);
return Some(ResolveValueResult::Partial(ty, 1, None));
return Some((
ResolveValueResult::Partial(ty, 1, None),
ResolvePathResultPrefixInfo::None,
));
}
}
&Scope::ImplDefScope(impl_) => {
if *first_name == sym::Self_.clone() {
return Some(ResolveValueResult::Partial(
TypeNs::SelfType(impl_),
1,
None,
return Some((
ResolveValueResult::Partial(TypeNs::SelfType(impl_), 1, None),
ResolvePathResultPrefixInfo::None,
));
}
}
Scope::AdtScope(adt) => {
if *first_name == sym::Self_.clone() {
let ty = TypeNs::AdtSelfType(*adt);
return Some(ResolveValueResult::Partial(ty, 1, None));
return Some((
ResolveValueResult::Partial(ty, 1, None),
ResolvePathResultPrefixInfo::None,
));
}
}
Scope::BlockScope(m) => {
@ -413,7 +439,10 @@ impl Resolver {
// `use core::u16;`.
if path.kind == PathKind::Plain && n_segments > 1 {
if let Some(builtin) = BuiltinType::by_name(first_name) {
return Some(ResolveValueResult::Partial(TypeNs::BuiltinType(builtin), 1, None));
return Some((
ResolveValueResult::Partial(TypeNs::BuiltinType(builtin), 1, None),
ResolvePathResultPrefixInfo::None,
));
}
}
@ -924,15 +953,15 @@ impl ModuleItemMap {
&self,
db: &dyn DefDatabase,
path: &ModPath,
) -> Option<ResolveValueResult> {
let (module_def, idx) =
) -> Option<(ResolveValueResult, ResolvePathResultPrefixInfo)> {
let (module_def, unresolved_idx, prefix_info) =
self.def_map.resolve_path_locally(db, self.module_id, path, BuiltinShadowMode::Other);
match idx {
match unresolved_idx {
None => {
let (value, import) = to_value_ns(module_def)?;
Some(ResolveValueResult::ValueNs(value, import))
Some((ResolveValueResult::ValueNs(value, import), prefix_info))
}
Some(idx) => {
Some(unresolved_idx) => {
let def = module_def.take_types_full()?;
let ty = match def.def {
ModuleDefId::AdtId(it) => TypeNs::AdtId(it),
@ -948,7 +977,7 @@ impl ModuleItemMap {
| ModuleDefId::MacroId(_)
| ModuleDefId::StaticId(_) => return None,
};
Some(ResolveValueResult::Partial(ty, idx, def.import))
Some((ResolveValueResult::Partial(ty, unresolved_idx, def.import), prefix_info))
}
}
}
@ -958,7 +987,7 @@ impl ModuleItemMap {
db: &dyn DefDatabase,
path: &ModPath,
) -> Option<(TypeNs, Option<usize>, Option<ImportOrExternCrate>)> {
let (module_def, idx) =
let (module_def, idx, _) =
self.def_map.resolve_path_locally(db, self.module_id, path, BuiltinShadowMode::Other);
let (res, import) = to_type_ns(module_def)?;
Some((res, idx, import))