Skip to content

Commit

Permalink
Auto merge of #127241 - cjgillot:def-collector-span, r=<try>
Browse files Browse the repository at this point in the history 
Mark span parent in def_collector.

The current device of marking spans with a parent def-id during lowering has been frustrating me for quite some time, as it's very easy to forget marking some spans.

This PR moves such marking to the def_collector, which is responsible for creating def-ids on partially expanded AST. This is much more robust as long as visitors are exhaustive.

r? ghost
  • Loading branch information
@bors
bors committed Jul 2, 2024
2 parents 7d97c59 + 95dc7f7 commit ec84e1c
Show file tree
Hide file tree
Showing 29 changed files with 633 additions and 485 deletions.
7 changes: 2 additions & 5 deletions compiler/rustc_ast/src/ast.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ pub enum GenericParamKind {
Const {
ty: P<Ty>,
/// Span of the `const` keyword.
kw_span: Span,
span: Span,
/// Optional default value for the const generic param
default: Option<AnonConst>,
},
Expand All @@ -375,10 +375,7 @@ impl GenericParam {
self.ident.span
}
GenericParamKind::Type { default: Some(ty) } => self.ident.span.to(ty.span),
GenericParamKind::Const { kw_span, default: Some(default), .. } => {
kw_span.to(default.value.span)
}
GenericParamKind::Const { kw_span, default: None, ty } => kw_span.to(ty.span),
GenericParamKind::Const { span, .. } => *span,
}
}
}
Expand Down
119 changes: 95 additions & 24 deletions compiler/rustc_ast/src/mut_visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ where
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
fn visit_attrs<T: MutVisitor>(attrs: &mut AttrVec, vis: &mut T) {
pub fn visit_attrs<T: MutVisitor>(attrs: &mut AttrVec, vis: &mut T) {
for attr in attrs.iter_mut() {
vis.visit_attribute(attr);
}
Expand All @@ -390,7 +390,7 @@ fn visit_bounds<T: MutVisitor>(bounds: &mut GenericBounds, vis: &mut T) {
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
fn visit_fn_sig<T: MutVisitor>(FnSig { header, decl, span }: &mut FnSig, vis: &mut T) {
pub fn visit_fn_sig<T: MutVisitor>(FnSig { header, decl, span }: &mut FnSig, vis: &mut T) {
vis.visit_fn_header(header);
vis.visit_fn_decl(decl);
vis.visit_span(span);
Expand Down Expand Up @@ -637,7 +637,7 @@ fn noop_visit_local<T: MutVisitor>(local: &mut P<Local>, vis: &mut T) {
vis.visit_span(span);
}

fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
pub fn noop_visit_attribute<T: MutVisitor>(attr: &mut Attribute, vis: &mut T) {
let Attribute { kind, id: _, style: _, span } = attr;
match kind {
AttrKind::Normal(normal) => {
Expand Down Expand Up @@ -836,7 +836,7 @@ fn visit_nonterminal<T: MutVisitor>(nt: &mut token::Nonterminal, vis: &mut T) {
}

// No `noop_` prefix because there isn't a corresponding method in `MutVisitor`.
fn visit_defaultness<T: MutVisitor>(defaultness: &mut Defaultness, vis: &mut T) {
pub fn visit_defaultness<T: MutVisitor>(defaultness: &mut Defaultness, vis: &mut T) {
match defaultness {
Defaultness::Default(span) => vis.visit_span(span),
Defaultness::Final => {}
Expand Down Expand Up @@ -871,7 +871,8 @@ fn visit_constness<T: MutVisitor>(constness: &mut Const, vis: &mut T) {
fn noop_visit_closure_binder<T: MutVisitor>(binder: &mut ClosureBinder, vis: &mut T) {
match binder {
ClosureBinder::NotPresent => {}
ClosureBinder::For { span: _, generic_params } => {
ClosureBinder::For { span, generic_params } => {
vis.visit_span(span);
generic_params.flat_map_in_place(|param| vis.flat_map_generic_param(param));
}
}
Expand Down Expand Up @@ -904,7 +905,10 @@ fn noop_visit_fn_ret_ty<T: MutVisitor>(fn_ret_ty: &mut FnRetTy, vis: &mut T) {

fn noop_visit_param_bound<T: MutVisitor>(pb: &mut GenericBound, vis: &mut T) {
match pb {
GenericBound::Trait(ty, _modifier) => vis.visit_poly_trait_ref(ty),
GenericBound::Trait(ty, modifier) => {
vis.visit_poly_trait_ref(ty);
visit_trait_bound_modifier(modifier, vis);
}
GenericBound::Outlives(lifetime) => noop_visit_lifetime(lifetime, vis),
GenericBound::Use(args, span) => {
for arg in args {
Expand All @@ -915,6 +919,22 @@ fn noop_visit_param_bound<T: MutVisitor>(pb: &mut GenericBound, vis: &mut T) {
}
}

fn visit_trait_bound_modifier<T: MutVisitor>(tbm: &mut TraitBoundModifiers, vis: &mut T) {
let TraitBoundModifiers { constness, asyncness, polarity } = tbm;
match constness {
BoundConstness::Never => {}
BoundConstness::Always(span) | BoundConstness::Maybe(span) => vis.visit_span(span),
}
match asyncness {
BoundAsyncness::Normal => {}
BoundAsyncness::Async(span) => vis.visit_span(span),
}
match polarity {
BoundPolarity::Positive => {}
BoundPolarity::Negative(span) | BoundPolarity::Maybe(span) => vis.visit_span(span),
}
}

fn noop_visit_precise_capturing_arg<T: MutVisitor>(arg: &mut PreciseCapturingArg, vis: &mut T) {
match arg {
PreciseCapturingArg::Lifetime(lt) => {
Expand All @@ -941,8 +961,9 @@ pub fn noop_flat_map_generic_param<T: MutVisitor>(
GenericParamKind::Type { default } => {
visit_opt(default, |default| vis.visit_ty(default));
}
GenericParamKind::Const { ty, kw_span: _, default } => {
GenericParamKind::Const { ty, span, default } => {
vis.visit_ty(ty);
vis.visit_span(span);
visit_opt(default, |default| vis.visit_anon_const(default));
}
}
Expand Down Expand Up @@ -1368,21 +1389,24 @@ pub fn noop_visit_pat<T: MutVisitor>(pat: &mut P<Pat>, vis: &mut T) {
vis.visit_span(span);
}

fn noop_visit_anon_const<T: MutVisitor>(AnonConst { id, value }: &mut AnonConst, vis: &mut T) {
pub fn noop_visit_anon_const<T: MutVisitor>(AnonConst { id, value }: &mut AnonConst, vis: &mut T) {
vis.visit_id(id);
vis.visit_expr(value);
}

fn noop_visit_inline_asm<T: MutVisitor>(asm: &mut InlineAsm, vis: &mut T) {
// FIXME: Visit spans inside all this currently ignored stuff.
let InlineAsm {
template: _,
template_strs: _,
operands,
clobber_abis: _,
options: _,
line_spans: _,
} = asm;
let InlineAsm { template, template_strs, operands, clobber_abis, options: _, line_spans } = asm;
for piece in template.iter_mut() {
match piece {
InlineAsmTemplatePiece::String(_str) => {}
InlineAsmTemplatePiece::Placeholder { operand_idx: _, modifier: _, span } => {
vis.visit_span(span)
}
}
}
for (_s1, _s2, span) in template_strs.iter_mut() {
vis.visit_span(span)
}
for (op, span) in operands {
match op {
InlineAsmOperand::In { expr, reg: _ }
Expand All @@ -1401,6 +1425,12 @@ fn noop_visit_inline_asm<T: MutVisitor>(asm: &mut InlineAsm, vis: &mut T) {
}
vis.visit_span(span);
}
for (_s1, span) in clobber_abis.iter_mut() {
vis.visit_span(span)
}
for span in line_spans.iter_mut() {
vis.visit_span(span)
}
}

fn noop_visit_inline_asm_sym<T: MutVisitor>(
Expand All @@ -1413,8 +1443,7 @@ fn noop_visit_inline_asm_sym<T: MutVisitor>(
}

fn noop_visit_format_args<T: MutVisitor>(fmt: &mut FormatArgs, vis: &mut T) {
// FIXME: visit the template exhaustively.
let FormatArgs { span, template: _, arguments } = fmt;
let FormatArgs { span, template, arguments } = fmt;
for FormatArgument { kind, expr } in arguments.all_args_mut() {
match kind {
FormatArgumentKind::Named(ident) | FormatArgumentKind::Captured(ident) => {
Expand All @@ -1424,9 +1453,48 @@ fn noop_visit_format_args<T: MutVisitor>(fmt: &mut FormatArgs, vis: &mut T) {
}
vis.visit_expr(expr);
}
for piece in template.iter_mut() {
match piece {
FormatArgsPiece::Literal(_symbol) => {}
FormatArgsPiece::Placeholder(placeholder) => visit_format_placeholder(placeholder, vis),
}
}
vis.visit_span(span);
}

fn visit_format_placeholder<T: MutVisitor>(
FormatPlaceholder { argument, span, format_options, format_trait: _ }: &mut FormatPlaceholder,
vis: &mut T,
) {
visit_opt(span, |span| vis.visit_span(span));
let FormatArgPosition { span, index: _, kind: _ } = argument;
visit_opt(span, |span| vis.visit_span(span));
let FormatOptions {
width,
precision,
alignment: _,
fill: _,
sign: _,
alternate: _,
zero_pad: _,
debug_hex: _,
} = format_options;
match width {
None => {}
Some(FormatCount::Literal(_)) => {}
Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => {
visit_opt(span, |span| vis.visit_span(span))
}
}
match precision {
None => {}
Some(FormatCount::Literal(_)) => {}
Some(FormatCount::Argument(FormatArgPosition { span, index: _, kind: _ })) => {
visit_opt(span, |span| vis.visit_span(span))
}
}
}

pub fn noop_visit_expr<T: MutVisitor>(
Expr { kind, id, span, attrs, tokens }: &mut Expr,
vis: &mut T,
Expand Down Expand Up @@ -1460,7 +1528,8 @@ pub fn noop_visit_expr<T: MutVisitor>(
visit_thin_exprs(call_args, vis);
vis.visit_span(span);
}
ExprKind::Binary(_binop, lhs, rhs) => {
ExprKind::Binary(Spanned { node: _binop, span }, lhs, rhs) => {
vis.visit_span(span);
vis.visit_expr(lhs);
vis.visit_expr(rhs);
}
Expand Down Expand Up @@ -1528,9 +1597,10 @@ pub fn noop_visit_expr<T: MutVisitor>(
visit_opt(label, |label| vis.visit_label(label));
vis.visit_block(blk);
}
ExprKind::Gen(_capture_by, body, _kind, decl_span) => {
ExprKind::Gen(capture_clause, body, _kind, decl_span) => {
vis.visit_block(body);
vis.visit_span(decl_span);
vis.visit_capture_by(capture_clause);
}
ExprKind::Await(expr, await_kw_span) => {
vis.visit_expr(expr);
Expand All @@ -1541,7 +1611,8 @@ pub fn noop_visit_expr<T: MutVisitor>(
vis.visit_expr(er);
vis.visit_span(span);
}
ExprKind::AssignOp(_op, el, er) => {
ExprKind::AssignOp(Spanned { node: _binop, span }, el, er) => {
vis.visit_span(span);
vis.visit_expr(el);
vis.visit_expr(er);
}
Expand Down Expand Up @@ -1593,7 +1664,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
fields.flat_map_in_place(|field| vis.flat_map_expr_field(field));
match rest {
StructRest::Base(expr) => vis.visit_expr(expr),
StructRest::Rest(_span) => {}
StructRest::Rest(span) => vis.visit_span(span),
StructRest::None => {}
}
}
Expand Down Expand Up @@ -1626,6 +1697,7 @@ pub fn noop_flat_map_stmt<T: MutVisitor>(
vis: &mut T,
) -> SmallVec<[Stmt; 1]> {
vis.visit_id(&mut id);
vis.visit_span(&mut span);
let stmts: SmallVec<_> = noop_flat_map_stmt_kind(kind, vis)
.into_iter()
.map(|kind| Stmt { id, kind, span })
Expand All @@ -1636,7 +1708,6 @@ pub fn noop_flat_map_stmt<T: MutVisitor>(
the visitor should implement custom statement visiting"
);
}
vis.visit_span(&mut span);
stmts
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/visit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ pub fn walk_generic_param<'a, V: Visitor<'a>>(
match kind {
GenericParamKind::Lifetime => (),
GenericParamKind::Type { default } => visit_opt!(visitor, visit_ty, default),
GenericParamKind::Const { ty, default, kw_span: _ } => {
GenericParamKind::Const { ty, default, span: _ } => {
try_visit!(visitor.visit_ty(ty));
visit_opt!(visitor, visit_anon_const, default);
}
Expand Down
11 changes: 4 additions & 7 deletions compiler/rustc_ast_lowering/src/asm.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
hir::InlineAsmOperand::Label { block: self.lower_block(block, false) }
}
};
(op, self.lower_span(*op_sp))
(op, *op_sp)
})
.collect();

Expand Down Expand Up @@ -458,7 +458,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
late: true,
expr: None,
},
self.lower_span(abi_span),
abi_span,
));
clobbered.insert(clobber);
}
Expand All @@ -468,12 +468,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let operands = self.arena.alloc_from_iter(operands);
let template = self.arena.alloc_from_iter(asm.template.iter().cloned());
let template_strs = self.arena.alloc_from_iter(
asm.template_strs
.iter()
.map(|(sym, snippet, span)| (*sym, *snippet, self.lower_span(*span))),
asm.template_strs.iter().map(|(sym, snippet, span)| (*sym, *snippet, *span)),
);
let line_spans =
self.arena.alloc_from_iter(asm.line_spans.iter().map(|span| self.lower_span(*span)));
let line_spans = self.arena.alloc_from_iter(asm.line_spans.iter().copied());
let hir_asm =
hir::InlineAsm { template, template_strs, operands, options: asm.options, line_spans };
self.arena.alloc(hir_asm)
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_ast_lowering/src/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let (stmts, expr) = self.lower_stmts(&b.stmts);
let rules = self.lower_block_check_mode(&b.rules);
let hir_id = self.lower_node_id(b.id);
hir::Block { hir_id, stmts, expr, rules, span: self.lower_span(b.span), targeted_by_break }
hir::Block { hir_id, stmts, expr, rules, span: b.span, targeted_by_break }
}

fn lower_stmts(
Expand All @@ -37,7 +37,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let local = self.lower_local(local);
self.alias_attrs(hir_id, local.hir_id);
let kind = hir::StmtKind::Let(local);
let span = self.lower_span(s.span);
let span = s.span;
stmts.push(hir::Stmt { hir_id, kind, span });
}
StmtKind::Item(it) => {
Expand All @@ -48,7 +48,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
_ => self.next_id(),
};
let kind = hir::StmtKind::Item(item_id);
let span = self.lower_span(s.span);
let span = s.span;
hir::Stmt { hir_id, kind, span }
},
));
Expand All @@ -61,7 +61,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let hir_id = self.lower_node_id(s.id);
self.alias_attrs(hir_id, e.hir_id);
let kind = hir::StmtKind::Expr(e);
let span = self.lower_span(s.span);
let span = s.span;
stmts.push(hir::Stmt { hir_id, kind, span });
}
}
Expand All @@ -70,7 +70,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
let hir_id = self.lower_node_id(s.id);
self.alias_attrs(hir_id, e.hir_id);
let kind = hir::StmtKind::Semi(e);
let span = self.lower_span(s.span);
let span = s.span;
stmts.push(hir::Stmt { hir_id, kind, span });
}
StmtKind::Empty => {}
Expand All @@ -94,7 +94,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
} else {
None
};
let span = self.lower_span(l.span);
let span = l.span;
let source = hir::LocalSource::Normal;
self.lower_attrs(hir_id, &l.attrs);
self.arena.alloc(hir::LetStmt { hir_id, ty, pat, init, els, span, source })
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/delegation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
delegation: &Delegation,
item_id: NodeId,
) -> DelegationResults<'hir> {
let span = self.lower_span(delegation.path.segments.last().unwrap().ident.span);
let span = delegation.path.segments.last().unwrap().ident.span;
let sig_id = self.get_delegation_sig_id(item_id, delegation.id, span);
match sig_id {
Ok(sig_id) => {
Expand Down
Loading

0 comments on commit ec84e1c

Please sign in to comment.