Skip to content

Select proper span for delegation, update span of child path segment if needed #154002

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

Closed
aerooneqq wants to merge 1 commit into from
+41 −17
Closed

Select proper span for delegation, update span of child path segment if needed #154002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
44 changes: 34 additions & 10 deletions compiler/rustc_ast_lowering/src/delegation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,8 +154,26 @@ impl<'hir> LoweringContext<'_, 'hir> {
&mut self,
delegation: &Delegation,
item_id: NodeId,
item_span: Span,
) -> DelegationResults<'hir> {
let span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
let last_seg_span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
let item_span = self.lower_span(item_span);

// Check if the span of the last segment is contained in item span. The span of last segment
// may refer to completely different entity, for example when we do a glob reuse we
// generate segments (in `rustc_expand::expand::build_single_delegations`),
// and during `rustc_resolve::macros::glob_delegation_suffixes` we generate
// suffixes, where identifiers that are used in generated segment
// use spans of the original functions from trait,
// thus those spans can point either to local or external
// trait functions. This `span` is also used for diagnostics purposes, so we want it to point
// to delegation, not to the random trait function, so we perform this check here.
// We could have added similar check in `rustc_resolve::macros::glob_delegation_suffixes`
Copy link
Contributor

@petrochenkov petrochenkov Mar 17, 2026

Choose a reason for hiding this comment

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

This was my first reaction - we should generate a good span (not referring to a completely different entity) during expansion, when we are building the expanded delegation item AST. Patching these spans during AST lowering doesn't seem right.

What exactly breaks if impl-reuse-pass if the span is changed during expansion?

Copy link
Contributor

@petrochenkov petrochenkov Mar 17, 2026

Choose a reason for hiding this comment

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

There's also a question of which spans to use exactly.

  • For single delegations expanded from glob delegations I'd set the last segment's span to the star symbol's span.
  • For reuse impl there's no better choice than the whole item span, I guess.

Copy link
Contributor Author

@aerooneqq aerooneqq Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Errors are in macros test. I've tried to change span only for path segment (as in code below) and span of the ident, in both cases there were errors.

Changes 
suffixes.iter().map(move |&(ident, rename)| {
     let mut path = deleg.prefix.clone();
     let mut seg_ident = ident;
     if !item_span.contains(seg_ident.span) {
         seg_ident.span = item_span;
     }

     path.segments.push(ast::PathSegment {
         ident: seg_ident,
         id: ast::DUMMY_NODE_ID,
         args: None,
     });

     ast::Item {
         attrs: item.attrs.clone(),
         id: ast::DUMMY_NODE_ID,
         span: if from_glob { item_span } else { ident.span },
         vis: item.vis.clone(),
         kind: Node::delegation_item_kind(Box::new(ast::Delegation {
             id: ast::DUMMY_NODE_ID,
             qself: deleg.qself.clone(),
             path,
             ident: rename.unwrap_or(ident),
             rename,
             body: deleg.body.clone(),
             from_glob,
         })),
         tokens: None,
     }
 })
Errors 
error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:177:80
   |
LL |     macro_rules! one_line_reuse { ($self:ident) => { reuse impl Trait for S1 { $self.0 } } }
   |                                                      --------------------------^^^^^----
   |                                                      |                         |
   |                                                      |                         `self` value is a keyword only available in methods with a `self` parameter
   |                                                      `self` not allowed in an implementation
LL |     one_line_reuse!(self);
   |     --------------------- in this macro invocation
   |
   = note: this error originates in the macro `one_line_reuse` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:177:80
   |
LL |     macro_rules! one_line_reuse { ($self:ident) => { reuse impl Trait for S1 { $self.0 } } }
   |                                                      --------------------------^^^^^----
   |                                                      |                         |
   |                                                      |                         `self` value is a keyword only available in methods with a `self` parameter
   |                                                      `self` not allowed in an implementation
LL |     one_line_reuse!(self);
   |     --------------------- in this macro invocation
   |
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
   = note: this error originates in the macro `one_line_reuse` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:182:26
   |
LL |     macro_rules! one_line_reuse_expr { ($x:expr) => { reuse impl Trait for S2 { $x } } }
   |                                                       ------------------------------ `self` not allowed in an implementation
LL |     one_line_reuse_expr!(self.0);
   |                          ^^^^ `self` value is a keyword only available in methods with a `self` parameter

error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:182:26
   |
LL |     macro_rules! one_line_reuse_expr { ($x:expr) => { reuse impl Trait for S2 { $x } } }
   |                                                       ------------------------------ `self` not allowed in an implementation
LL |     one_line_reuse_expr!(self.0);
   |                          ^^^^ `self` value is a keyword only available in methods with a `self` parameter
   |
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:187:27
   |
LL |     macro_rules! one_line_reuse_expr2 { ($x:expr) => { reuse impl Trait for s3!() { $x } } }
   |                                                        --------------------------------- `self` not allowed in an implementation
LL |     one_line_reuse_expr2!(self.0);
   |                           ^^^^ `self` value is a keyword only available in methods with a `self` parameter

error[E0424]: expected value, found module `self`
  --> C:\work\rust\tests\ui\delegation\impl-reuse-pass.rs:187:27
   |
LL |     macro_rules! one_line_reuse_expr2 { ($x:expr) => { reuse impl Trait for s3!() { $x } } }
   |                                                        --------------------------------- `self` not allowed in an implementation
LL |     one_line_reuse_expr2!(self.0);
   |                           ^^^^ `self` value is a keyword only available in methods with a `self` parameter
   |
   = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 6 previous errors

Copy link
Contributor

@petrochenkov petrochenkov Mar 18, 2026

Choose a reason for hiding this comment

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

I'll debug the test failure, I'm sure that we should do it during macro expansion.

// (i.e. create ident with span related to ast path of delegation, not the original trait item),
// or in `rustc_expand::expand::build_single_delegations`,
// however it will break `impl-reuse-pass` test and checking span here seems to be good solution,
// as bad spans may come from different places in future.
let span = if item_span.contains(last_seg_span) { last_seg_span } else { item_span };

// Delegation can be unresolved in illegal places such as function bodies in extern blocks (see #151356)
let ids = if let Some(delegation_info) =
Expand Down Expand Up @@ -651,7 +669,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

// FIXME(fn_delegation): proper support for parent generics propagation
// in method call scenario.
let segment = self.process_segment(item_id, span, &segment, &mut generics.child, false);
let segment = self.process_segment(item_id, span, &segment, &mut generics.child, true);
let segment = self.arena.alloc(segment);

self.arena.alloc(hir::Expr {
Expand All @@ -677,14 +695,14 @@ impl<'hir> LoweringContext<'_, 'hir> {

new_path.segments = self.arena.alloc_from_iter(
new_path.segments.iter().enumerate().map(|(idx, segment)| {
let mut process_segment = |result, add_lifetimes| {
self.process_segment(item_id, span, segment, result, add_lifetimes)
let mut process_segment = |result, is_child| {
self.process_segment(item_id, span, segment, result, is_child)
};

if idx + 2 == len {
process_segment(&mut generics.parent, true)
process_segment(&mut generics.parent, false)
} else if idx + 1 == len {
process_segment(&mut generics.child, false)
process_segment(&mut generics.child, true)
} else {
segment.clone()
}
Expand All @@ -695,7 +713,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
hir::QPath::TypeRelative(ty, segment) => {
let segment =
self.process_segment(item_id, span, segment, &mut generics.child, false);
self.process_segment(item_id, span, segment, &mut generics.child, true);

hir::QPath::TypeRelative(ty, self.arena.alloc(segment))
}
Expand Down Expand Up @@ -723,17 +741,17 @@ impl<'hir> LoweringContext<'_, 'hir> {
span: Span,
segment: &hir::PathSegment<'hir>,
result: &mut GenericsGenerationResult<'hir>,
add_lifetimes: bool,
is_child_segment: bool,
) -> hir::PathSegment<'hir> {
let details = result.generics.args_propagation_details();

// The first condition is needed when there is SelfAndUserSpecified case,
// we don't want to propagate generics params in this situation.
let segment = if details.should_propagate
let mut segment = if details.should_propagate
&& let Some(args) = result
.generics
.into_hir_generics(self, item_id, span)
.into_generic_args(self, add_lifetimes, span)
.into_generic_args(self, !is_child_segment, span)
{
hir::PathSegment { args: Some(args), ..segment.clone() }
} else {
Expand All @@ -744,6 +762,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
result.args_segment_id = Some(segment.hir_id);
}

// Update segment ident span if it is not contained in delegation span, see comment
// in `lower_delegation` function in this file.
if is_child_segment && !span.contains(segment.ident.span) {
segment.ident.span = span;
}

segment
}

Expand Down
6 changes: 3 additions & 3 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 @@ -579,7 +579,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
hir::ItemKind::Macro(ident, macro_def, macro_kinds)
}
ItemKind::Delegation(box delegation) => {
let delegation_results = self.lower_delegation(delegation, id);
let delegation_results = self.lower_delegation(delegation, id, span);
hir::ItemKind::Fn {
sig: delegation_results.sig,
ident: delegation_results.ident,
Expand Down Expand Up @@ -1079,7 +1079,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
(*ident, generics, kind, ty.is_some())
}
AssocItemKind::Delegation(box delegation) => {
let delegation_results = self.lower_delegation(delegation, i.id);
let delegation_results = self.lower_delegation(delegation, i.id, i.span);
let item_kind = hir::TraitItemKind::Fn(
delegation_results.sig,
hir::TraitFn::Provided(delegation_results.body_id),
Expand Down Expand Up @@ -1272,7 +1272,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
)
}
AssocItemKind::Delegation(box delegation) => {
let delegation_results = self.lower_delegation(delegation, i.id);
let delegation_results = self.lower_delegation(delegation, i.id, i.span);
(
delegation.ident,
(
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/delegation/impl-reuse-negative-traits.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

trait Trait {
fn foo(&self);
//~^ ERROR negative impls cannot have any items [E0749]
}

struct S;
Expand All @@ -15,5 +14,6 @@ impl Trait for S {
struct F(S);

reuse impl !Trait for F { &self.0 }
//~^ ERROR negative impls cannot have any items [E0749]

fn main() {}
6 changes: 3 additions & 3 deletions tests/ui/delegation/impl-reuse-negative-traits.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0749]: negative impls cannot have any items
--> $DIR/impl-reuse-negative-traits.rs:6:8
--> $DIR/impl-reuse-negative-traits.rs:16:1
|
LL | fn foo(&self);
| ^^^
LL | reuse impl !Trait for F { &self.0 }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to 1 previous error

Expand Down
Loading