Skip to content

Commit ccb2194

Browse files
nnethercotennethercote
committed
Factor some code out of AstValidator::visit_items.
Currently it uses `walk_item` on some item kinds. For other item kinds it visits the fields individually. For the latter group, this commit adds `visit_attrs_vis` and `visit_attrs_vis_ident` which bundle up visits to the fields that don't need special handling. This makes it clearer that they haven't been forgotten about. Also, it's better to do the attribute visits at the start because attributes precede the items in the source code. Because of this, a couple of tests have their output improved: errors appear in an order that matches the source code order.
1 parent 9bdac17 commit ccb2194

File tree

3 files changed

+52
-51
lines changed

3 files changed

+52
-51
lines changed

‎compiler/rustc_ast_passes/src/ast_validation.rs‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -727,6 +727,19 @@ impl<'a> AstValidator<'a> {
727727
)
728728
}
729729
}
730+
731+
// Used within `visit_item` for item kinds where we don't call `visit::walk_item`.
732+
fn visit_attrs_vis(&mut self, attrs: &'a AttrVec, vis: &'a Visibility) {
733+
walk_list!(self, visit_attribute, attrs);
734+
self.visit_vis(vis);
735+
}
736+
737+
// Used within `visit_item` for item kinds where we don't call `visit::walk_item`.
738+
fn visit_attrs_vis_ident(&mut self, attrs: &'a AttrVec, vis: &'a Visibility, ident: &'a Ident) {
739+
walk_list!(self, visit_attribute, attrs);
740+
self.visit_vis(vis);
741+
self.visit_ident(ident);
742+
}
730743
}
731744

732745
/// Checks that generic parameters are in the correct order,
@@ -834,6 +847,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
834847
self_ty,
835848
items,
836849
}) => {
850+
self.visit_attrs_vis(&item.attrs, &item.vis);
837851
self.with_in_trait_impl(Some((*constness, *polarity, t)), |this| {
838852
this.visibility_not_permitted(
839853
&item.vis,
@@ -853,7 +867,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
853867
});
854868
}
855869

856-
this.visit_vis(&item.vis);
857870
let disallowed = matches!(constness, Const::No)
858871
.then(|| TildeConstReason::TraitImpl { span: item.span });
859872
this.with_tilde_const(disallowed, |this| this.visit_generics(generics));
@@ -862,7 +875,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
862875

863876
walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: true });
864877
});
865-
walk_list!(self, visit_attribute, &item.attrs);
866878
}
867879
ItemKind::Impl(box Impl {
868880
safety,
@@ -882,6 +894,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
882894
only_trait,
883895
};
884896

897+
self.visit_attrs_vis(&item.attrs, &item.vis);
885898
self.with_in_trait_impl(None, |this| {
886899
this.visibility_not_permitted(
887900
&item.vis,
@@ -905,15 +918,13 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
905918
this.dcx().emit_err(error(span, "`const`", true));
906919
}
907920

908-
this.visit_vis(&item.vis);
909921
this.with_tilde_const(
910922
Some(TildeConstReason::Impl { span: item.span }),
911923
|this| this.visit_generics(generics),
912924
);
913925
this.visit_ty(self_ty);
914926
walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: false });
915927
});
916-
walk_list!(self, visit_attribute, &item.attrs);
917928
}
918929
ItemKind::Fn(
919930
func @ box Fn {
@@ -926,6 +937,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
926937
define_opaque: _,
927938
},
928939
) => {
940+
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
929941
self.check_defaultness(item.span, *defaultness);
930942

931943
let is_intrinsic =
@@ -953,11 +965,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
953965
});
954966
}
955967

956-
self.visit_vis(&item.vis);
957-
self.visit_ident(ident);
958968
let kind = FnKind::Fn(FnCtxt::Free, &item.vis, &*func);
959969
self.visit_fn(kind, item.span, item.id);
960-
walk_list!(self, visit_attribute, &item.attrs);
961970
}
962971
ItemKind::ForeignMod(ForeignMod { extern_span, abi, safety, .. }) => {
963972
self.with_in_extern_mod(*safety, |this| {
@@ -1005,6 +1014,7 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10051014
visit::walk_item(self, item)
10061015
}
10071016
ItemKind::Trait(box Trait { is_auto, generics, ident, bounds, items, .. }) => {
1017+
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
10081018
let is_const_trait =
10091019
attr::find_by_name(&item.attrs, sym::const_trait).map(|attr| attr.span);
10101020
self.with_in_trait(item.span, is_const_trait, |this| {
@@ -1018,8 +1028,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10181028

10191029
// Equivalent of `visit::walk_item` for `ItemKind::Trait` that inserts a bound
10201030
// context for the supertraits.
1021-
this.visit_vis(&item.vis);
1022-
this.visit_ident(ident);
10231031
let disallowed = is_const_trait
10241032
.is_none()
10251033
.then(|| TildeConstReason::Trait { span: item.span });
@@ -1029,7 +1037,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10291037
});
10301038
walk_list!(this, visit_assoc_item, items, AssocCtxt::Trait);
10311039
});
1032-
walk_list!(self, visit_attribute, &item.attrs);
10331040
}
10341041
ItemKind::Mod(safety, ident, mod_kind) => {
10351042
if let &Safety::Unsafe(span) = safety {
@@ -1045,12 +1052,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10451052
}
10461053
ItemKind::Struct(ident, vdata, generics) => match vdata {
10471054
VariantData::Struct { fields, .. } => {
1048-
self.visit_vis(&item.vis);
1049-
self.visit_ident(ident);
1055+
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
10501056
self.visit_generics(generics);
10511057
// Permit `Anon{Struct,Union}` as field type.
10521058
walk_list!(self, visit_struct_field_def, fields);
1053-
walk_list!(self, visit_attribute, &item.attrs);
10541059
}
10551060
_ => visit::walk_item(self, item),
10561061
},
@@ -1060,12 +1065,10 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10601065
}
10611066
match vdata {
10621067
VariantData::Struct { fields, .. } => {
1063-
self.visit_vis(&item.vis);
1064-
self.visit_ident(ident);
1068+
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
10651069
self.visit_generics(generics);
10661070
// Permit `Anon{Struct,Union}` as field type.
10671071
walk_list!(self, visit_struct_field_def, fields);
1068-
walk_list!(self, visit_attribute, &item.attrs);
10691072
}
10701073
_ => visit::walk_item(self, item),
10711074
}
@@ -1484,10 +1487,8 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
14841487
|| ctxt == AssocCtxt::Trait
14851488
|| matches!(func.sig.header.constness, Const::Yes(_)) =>
14861489
{
1487-
self.visit_vis(&item.vis);
1488-
self.visit_ident(&func.ident);
1490+
self.visit_attrs_vis_ident(&item.attrs, &item.vis, &func.ident);
14891491
let kind = FnKind::Fn(FnCtxt::Assoc(ctxt), &item.vis, &*func);
1490-
walk_list!(self, visit_attribute, &item.attrs);
14911492
self.visit_fn(kind, item.span, item.id);
14921493
}
14931494
AssocItemKind::Type(_) => {

‎tests/ui/coverage-attr/name-value.stderr‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,21 @@ LL - #[coverage = "off"]
4343
LL + #[coverage(on)]
4444
|
4545

46+
error: malformed `coverage` attribute input
47+
--> $DIR/name-value.rs:26:1
48+
|
49+
LL | #[coverage = "off"]
50+
| ^^^^^^^^^^^^^^^^^^^
51+
|
52+
help: the following are the possible correct uses
53+
|
54+
LL - #[coverage = "off"]
55+
LL + #[coverage(off)]
56+
|
57+
LL - #[coverage = "off"]
58+
LL + #[coverage(on)]
59+
|
60+
4661
error: malformed `coverage` attribute input
4762
--> $DIR/name-value.rs:29:5
4863
|
@@ -59,7 +74,7 @@ LL + #[coverage(on)]
5974
|
6075

6176
error: malformed `coverage` attribute input
62-
--> $DIR/name-value.rs:26:1
77+
--> $DIR/name-value.rs:35:1
6378
|
6479
LL | #[coverage = "off"]
6580
| ^^^^^^^^^^^^^^^^^^^
@@ -104,7 +119,7 @@ LL + #[coverage(on)]
104119
|
105120

106121
error: malformed `coverage` attribute input
107-
--> $DIR/name-value.rs:35:1
122+
--> $DIR/name-value.rs:50:1
108123
|
109124
LL | #[coverage = "off"]
110125
| ^^^^^^^^^^^^^^^^^^^
@@ -148,21 +163,6 @@ LL - #[coverage = "off"]
148163
LL + #[coverage(on)]
149164
|
150165

151-
error: malformed `coverage` attribute input
152-
--> $DIR/name-value.rs:50:1
153-
|
154-
LL | #[coverage = "off"]
155-
| ^^^^^^^^^^^^^^^^^^^
156-
|
157-
help: the following are the possible correct uses
158-
|
159-
LL - #[coverage = "off"]
160-
LL + #[coverage(off)]
161-
|
162-
LL - #[coverage = "off"]
163-
LL + #[coverage(on)]
164-
|
165-
166166
error: malformed `coverage` attribute input
167167
--> $DIR/name-value.rs:64:1
168168
|

‎tests/ui/coverage-attr/word-only.stderr‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,19 @@ LL | #[coverage(off)]
3737
LL | #[coverage(on)]
3838
| ++++
3939

40+
error: malformed `coverage` attribute input
41+
--> $DIR/word-only.rs:26:1
42+
|
43+
LL | #[coverage]
44+
| ^^^^^^^^^^^
45+
|
46+
help: the following are the possible correct uses
47+
|
48+
LL | #[coverage(off)]
49+
| +++++
50+
LL | #[coverage(on)]
51+
| ++++
52+
4053
error: malformed `coverage` attribute input
4154
--> $DIR/word-only.rs:29:5
4255
|
@@ -51,7 +64,7 @@ LL | #[coverage(on)]
5164
| ++++
5265

5366
error: malformed `coverage` attribute input
54-
--> $DIR/word-only.rs:26:1
67+
--> $DIR/word-only.rs:35:1
5568
|
5669
LL | #[coverage]
5770
| ^^^^^^^^^^^
@@ -90,7 +103,7 @@ LL | #[coverage(on)]
90103
| ++++
91104

92105
error: malformed `coverage` attribute input
93-
--> $DIR/word-only.rs:35:1
106+
--> $DIR/word-only.rs:50:1
94107
|
95108
LL | #[coverage]
96109
| ^^^^^^^^^^^
@@ -128,19 +141,6 @@ LL | #[coverage(off)]
128141
LL | #[coverage(on)]
129142
| ++++
130143

131-
error: malformed `coverage` attribute input
132-
--> $DIR/word-only.rs:50:1
133-
|
134-
LL | #[coverage]
135-
| ^^^^^^^^^^^
136-
|
137-
help: the following are the possible correct uses
138-
|
139-
LL | #[coverage(off)]
140-
| +++++
141-
LL | #[coverage(on)]
142-
| ++++
143-
144144
error: malformed `coverage` attribute input
145145
--> $DIR/word-only.rs:64:1
146146
|

0 commit comments

Comments
 (0)