Skip to content

Some ast lowering partial-res cleanups #158948

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.

Sign up for GitHub

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

Open
oli-obk wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Some ast lowering partial-res cleanups #158948

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0553400
Add a get_full_res helper method for a common pattern
oli-obk Jul 8, 2026
4c31dbb
Avoid some query lookups for information that is already available
oli-obk Jul 8, 2026
b93d450
Use expect_full_res in more places
oli-obk Jul 8, 2026
389ab1d
Cleanups and documentation
oli-obk Jul 8, 2026
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
12 changes: 3 additions & 9 deletions compiler/rustc_ast_lowering/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,15 +201,9 @@ impl<'hir> LoweringContext<'_, 'hir> {
anon_const: self.lower_const_block(anon_const),
},
InlineAsmOperand::Sym { sym } => {
let static_def_id = self
.get_partial_res(sym.id)
.and_then(|res| res.full_res())
.and_then(|res| match res {
Res::Def(DefKind::Static { .. }, def_id) => Some(def_id),
_ => None,
});

if let Some(def_id) = static_def_id {
if let Some(Res::Def(DefKind::Static { .. }, def_id)) =
self.get_full_res(sym.id)
{
let path = self.lower_qpath(
sym.id,
&sym.qself,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/delegation/generics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,8 @@ impl<'hir> DelegationResolver<'_, 'hir> {
let qself_is_none = delegation.qself.is_none();

let parent_args = if let [.., parent_segment, _] = &delegation.path.segments[..] {
let res = self.get_resolution_id(parent_segment.id)?;
if matches!(tcx.def_kind(res), DefKind::Trait | DefKind::TraitAlias) {
let (_, kind) = self.get_resolution(parent_segment.id)?;
if matches!(kind, DefKind::Trait | DefKind::TraitAlias) {
sig_parent_params = &tcx.generics_of(sig_parent).own_params;
self.get_user_args(parent_segment)
.map(|args| ParentSegmentArgs::Specified(args))
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_ast_lowering/src/delegation/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,8 +589,7 @@ struct SelfResolver<'a, 'b, 'hir> {

impl SelfResolver<'_, '_, '_> {
fn try_replace_id(&mut self, id: NodeId) {
if let Some(res) = self.ctxt.get_partial_res(id)
&& let Some(Res::Local(sig_id)) = res.full_res()
if let Some(Res::Local(sig_id)) = self.ctxt.get_full_res(id)
&& sig_id == self.path_id
{
self.overwrites.push(id);
Expand Down
19 changes: 12 additions & 7 deletions compiler/rustc_ast_lowering/src/delegation/resolution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub(super) struct DelegationResolution {

pub(super) mod resolver {
use rustc_ast::NodeId;
use rustc_hir::def::{DefKind, Res};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_middle::ty::TyCtxt;
use rustc_span::ErrorGuaranteed;
Expand Down Expand Up @@ -95,10 +96,14 @@ pub(super) mod resolver {
}

#[inline]
pub(crate) fn get_resolution_id(&self, id: NodeId) -> Result<DefId, ErrorGuaranteed> {
self.0.get_partial_res(id).and_then(|r| r.expect_full_res().opt_def_id()).ok_or_else(
|| self.tcx().dcx().delayed_bug(format!("failed to resolve node {id:?}")),
)
pub(crate) fn get_resolution(
&self,
id: NodeId,
) -> Result<(DefId, DefKind), ErrorGuaranteed> {
let Res::Def(kind, id) = self.0.expect_full_res(id) else {
return Err(self.tcx().dcx().delayed_bug(format!("failed to resolve node {id:?}")));
};
Ok((id, kind))
}
}
}
Expand Down Expand Up @@ -149,7 +154,7 @@ impl<'tcx> DelegationResolver<'_, 'tcx> {
// FIXME(splat): use `sig.splatted()` once FnSig has it
param_info: ParamInfo { param_count, c_variadic: sig.c_variadic(), splatted: None },
source: delegation.source,
call_path_res: self.get_resolution_id(delegation.id)?,
call_path_res: self.get_resolution(delegation.id).map(|(id, _)| id)?,
sig_mapping: self.create_self_mapping(
delegation,
span,
Expand Down Expand Up @@ -306,7 +311,7 @@ impl<'tcx> DelegationResolver<'_, 'tcx> {
// Check that delegation path resolves to a trait AssocFn, not to a free method.
// After previous check we are sure that `sig_id` and `delegation.id`
// point to the same function.
let id = self.get_resolution_id(delegation.id)?;
Ok(tcx.def_kind(id) == DefKind::AssocFn && tcx.def_kind(tcx.parent(id)) == DefKind::Trait)
let (id, kind) = self.get_resolution(delegation.id)?;
Ok(kind == DefKind::AssocFn && tcx.def_kind(tcx.parent(id)) == DefKind::Trait)
}
}
20 changes: 8 additions & 12 deletions compiler/rustc_ast_lowering/src/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1226,12 +1226,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> Option<(&'a Option<Box<QSelf>>, &'a Path)> {
if let ExprKind::Path(qself, path) = &expr.kind {
// Does the path resolve to something disallowed in a tuple struct/variant pattern?
if let Some(partial_res) = self.get_partial_res(expr.id) {
if let Some(res) = partial_res.full_res()
&& !res.expected_in_tuple_struct_pat()
{
return None;
}
if let Some(res) = self.get_full_res(expr.id)
&& !res.expected_in_tuple_struct_pat()
{
return None;
}
return Some((qself, path));
}
Expand All @@ -1248,12 +1246,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
) -> Option<(&'a Option<Box<QSelf>>, &'a Path)> {
if let ExprKind::Path(qself, path) = &expr.kind {
// Does the path resolve to something disallowed in a unit struct/variant pattern?
if let Some(partial_res) = self.get_partial_res(expr.id) {
if let Some(res) = partial_res.full_res()
&& !res.expected_in_unit_struct_pat()
{
return None;
}
if let Some(res) = self.get_full_res(expr.id)
&& !res.expected_in_unit_struct_pat()
{
return None;
}
return Some((qself, path));
}
Expand Down
24 changes: 9 additions & 15 deletions compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,8 +583,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}

fn lower_path_simple_eii(&mut self, id: NodeId, path: &Path) -> Option<DefId> {
let res = self.get_partial_res(id)?;
let Some(did) = res.expect_full_res().opt_def_id() else {
let Some(did) = self.expect_full_res(id).opt_def_id() else {
self.dcx().span_delayed_bug(path.span, "should have errored in resolve");
return None;
};
Expand Down Expand Up @@ -1280,13 +1279,9 @@ impl<'hir> LoweringContext<'_, 'hir> {

let span = self.lower_span(i.span);
let (effective_ident, impl_kind) = if is_in_trait_impl {
let trait_item_def_id = self
.get_partial_res(i.id)
.and_then(|r| r.expect_full_res().opt_def_id())
.ok_or_else(|| {
self.dcx()
.span_delayed_bug(span, "could not resolve trait item being implemented")
});
let trait_item_def_id = self.expect_full_res(i.id).opt_def_id().ok_or_else(|| {
self.dcx().span_delayed_bug(span, "could not resolve trait item being implemented")
});
let effective_ident = self.check_pin_drop_sugar_impl_item(i, ident, trait_item_def_id);
(effective_ident, ImplItemImplKind::Trait { defaultness, trait_item_def_id })
} else {
Expand Down Expand Up @@ -1802,8 +1797,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
let kind = match &r.kind {
RestrictionKind::Unrestricted => hir::RestrictionKind::Unrestricted,
RestrictionKind::Restricted { path, id, shorthand: _ } => {
let res = self.get_partial_res(*id);
if let Some(did) = res.and_then(|res| res.expect_full_res().opt_def_id()) {
let res = self.expect_full_res(*id);
if let Some(did) = res.opt_def_id() {
hir::RestrictionKind::Restricted(self.arena.alloc(hir::Path {
res: did,
segments: self.arena.alloc_from_iter(path.segments.iter().map(|segment| {
Expand Down Expand Up @@ -1910,8 +1905,8 @@ impl<'hir> LoweringContext<'_, 'hir> {
return;
};
let define_opaque = define_opaque.iter().filter_map(|(id, path)| {
let res = self.get_partial_res(*id);
let Some(did) = res.and_then(|res| res.expect_full_res().opt_def_id()) else {
let res = self.expect_full_res(*id);
let Some(did) = res.opt_def_id() else {
self.dcx().span_delayed_bug(path.span, "should have errored in resolve");
return None;
};
Expand Down Expand Up @@ -2013,8 +2008,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
bounds,
}) => {
let rbp = if bound_generic_params.is_empty()
&& let Some(res) =
self.get_partial_res(bounded_ty.id).and_then(|r| r.full_res())
&& let Some(res) = self.get_full_res(bounded_ty.id)
&& let Res::Def(DefKind::TyParam, def_id) = res
&& params.iter().any(|p| def_id == self.local_def_id(p.id).to_def_id())
{
Expand Down
29 changes: 17 additions & 12 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,6 +812,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}

fn get_full_res(&self, id: NodeId) -> Option<Res<NodeId>> {
self.get_partial_res(id)?.full_res()

@petrochenkov petrochenkov Jul 9, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The behavior here is different from expect_full_res.

get_partial_res can return None in two cases:

  • id does not refer to any kind of path, we'd ideally panic in this case, this should not happen.
  • id refers to some unsuccessfully resolved path, we do not always put Res::Err for such paths into the table, unfortunately.
    That's why fn expect_full_res defaults to Res::Err if it doesn't find anything in the table. That is better than what this function does, because Res::Err is a full resolution. But it is also fishy because it creates Res::Err without ErrorGuaranteed, ideally we'd run delayed_bug if that fallback happens.

View changes since the review

}

/// Unless you're manually handling path resolutions, you do not want to use this function.
/// Prefer [Self::get_full_res] or [Self::expect_full_res] instead.
fn get_partial_res(&self, id: NodeId) -> Option<PartialRes> {
match self.partial_res_overrides.get(&id) {
Some(self_param_id) => Some(PartialRes::new(Res::Local(*self_param_id))),
Expand Down Expand Up @@ -1010,7 +1016,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
res.unwrap_or(Res::Err)
}

fn expect_full_res(&mut self, id: NodeId) -> Res<NodeId> {
/// The common entry point to obtaining resolution information.
///
/// ## Panics
///
/// If the resolution still has unresolved segments, this function will panic.
/// So lowering paths always needs to go through `get_full_res`.
fn expect_full_res(&self, id: NodeId) -> Res<NodeId> {
self.get_partial_res(id).map_or(Res::Err, |pr| pr.expect_full_res())
}

Expand Down Expand Up @@ -1460,10 +1472,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
//
// FIXME: Should we be handling `(PATH_TO_CONST)`?
TyKind::Path(None, path) => {
if let Some(res) = self
.get_partial_res(ty.id)
.and_then(|partial_res| partial_res.full_res())
{
if let Some(res) = self.get_full_res(ty.id) {
if !res.matches_ns(Namespace::TypeNS)
&& path.is_potential_trivial_const_arg()
{
Expand Down Expand Up @@ -1511,8 +1520,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
// The other cases when a qpath should be opportunistically made a trait object are handled
// by `ty_path`.
if qself.is_none()
&& let Some(partial_res) = self.get_partial_res(t.id)
&& let Some(Res::Def(DefKind::Trait | DefKind::TraitAlias, _)) = partial_res.full_res()
&& let Some(Res::Def(DefKind::Trait | DefKind::TraitAlias, _)) = self.get_full_res(t.id)
{
let (bounds, lifetime_bound) = self.with_dyn_type_scope(true, |this| {
let bound = this.lower_poly_trait_ref(
Expand Down Expand Up @@ -1877,9 +1885,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let [segment] = path.segments.as_slice() else {
panic!();
};
let res = self.get_partial_res(*id).map_or(Res::Err, |partial_res| {
partial_res.full_res().expect("no partial res expected for precise capture arg")
});
let res = self.expect_full_res(*id);
hir::PreciseCapturingArg::Param(hir::PreciseCapturingNonLifetimeArg {
hir_id: self.lower_node_id(*id),
ident: self.lower_ident(segment.ident),
Expand Down Expand Up @@ -2890,8 +2896,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
&anon.value
};

let maybe_res =
self.get_partial_res(expr.id).and_then(|partial_res| partial_res.full_res());
let maybe_res = self.get_full_res(expr.id);
if let ExprKind::Path(qself, path) = &expr.kind
&& path.is_potential_trivial_const_arg()
&& matches!(maybe_res, Some(Res::Def(DefKind::ConstParam, _)))
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_ast_lowering/src/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir_id: hir::HirId,
lower_sub: impl FnOnce(&mut Self) -> Option<&'hir hir::Pat<'hir>>,
) -> hir::PatKind<'hir> {
match self.get_partial_res(p.id).map(|d| d.expect_full_res()) {
// `None` can occur in body-less function signatures
res @ (None | Some(Res::Local(_))) => {
let res = self.expect_full_res(p.id);
match res {
// `Err` can occur in body-less function signatures
Res::Local(_) | Res::Err => {
let binding_id = match res {
Some(Res::Local(id)) => {
Res::Local(id) => {
// In `Or` patterns like `VariantA(s) | VariantB(s, _)`, multiple identifier patterns
// will be resolved to the same `Res::Local`. Thus they just share a single
// `HirId`.
Expand All @@ -318,7 +319,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
lower_sub(self),
)
}
Some(res) => {
res => {
let res = self.lower_res(res);
let span = self.lower_span(ident.span);
hir::PatKind::Expr(self.arena.alloc(hir::PatExpr {
Expand Down
Loading