Skip to content

Commit 2e7de1a

Browse files
committed
Reduce scope of AstValidator::with_* calls.
`AstValidator` has several `with_*` methods, each one setting a field that adjust how checking takes place for items within certain other items. E.g. `with_in_trait_impl` is used to adjust the checking done on items inside an `impl` item. Weirdly, the scopes used for most of the `with_*` calls are very broad, and include things that aren't "inside" the item, such as visibility, unsafety, and constness. This commit minimizes the scope of these `with_*` calls so they only apply to the things inside the item.
1 parent fb01485 commit 2e7de1a

File tree

1 file changed

+88
-91
lines changed

1 file changed

+88
-91
lines changed

Diff for: compiler/rustc_ast_passes/src/ast_validation.rs

+88-91
Original file line numberDiff line numberDiff line change
@@ -855,31 +855,30 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
855855
items,
856856
}) => {
857857
self.visit_attrs_vis(&item.attrs, &item.vis);
858-
self.with_in_trait_impl(Some((*constness, *polarity, t)), |this| {
859-
this.visibility_not_permitted(
860-
&item.vis,
861-
errors::VisibilityNotPermittedNote::TraitImpl,
862-
);
863-
if let TyKind::Dummy = self_ty.kind {
864-
// Abort immediately otherwise the `TyKind::Dummy` will reach HIR lowering,
865-
// which isn't allowed. Not a problem for this obscure, obsolete syntax.
866-
this.dcx().emit_fatal(errors::ObsoleteAuto { span: item.span });
867-
}
868-
if let (&Safety::Unsafe(span), &ImplPolarity::Negative(sp)) = (safety, polarity)
869-
{
870-
this.dcx().emit_err(errors::UnsafeNegativeImpl {
871-
span: sp.to(t.path.span),
872-
negative: sp,
873-
r#unsafe: span,
874-
});
875-
}
858+
self.visibility_not_permitted(
859+
&item.vis,
860+
errors::VisibilityNotPermittedNote::TraitImpl,
861+
);
862+
if let TyKind::Dummy = self_ty.kind {
863+
// Abort immediately otherwise the `TyKind::Dummy` will reach HIR lowering,
864+
// which isn't allowed. Not a problem for this obscure, obsolete syntax.
865+
self.dcx().emit_fatal(errors::ObsoleteAuto { span: item.span });
866+
}
867+
if let (&Safety::Unsafe(span), &ImplPolarity::Negative(sp)) = (safety, polarity) {
868+
self.dcx().emit_err(errors::UnsafeNegativeImpl {
869+
span: sp.to(t.path.span),
870+
negative: sp,
871+
r#unsafe: span,
872+
});
873+
}
876874

877-
let disallowed = matches!(constness, Const::No)
878-
.then(|| TildeConstReason::TraitImpl { span: item.span });
879-
this.with_tilde_const(disallowed, |this| this.visit_generics(generics));
880-
this.visit_trait_ref(t);
881-
this.visit_ty(self_ty);
875+
let disallowed = matches!(constness, Const::No)
876+
.then(|| TildeConstReason::TraitImpl { span: item.span });
877+
self.with_tilde_const(disallowed, |this| this.visit_generics(generics));
878+
self.visit_trait_ref(t);
879+
self.visit_ty(self_ty);
882880

881+
self.with_in_trait_impl(Some((*constness, *polarity, t)), |this| {
883882
walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: true });
884883
});
885884
}
@@ -902,34 +901,33 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
902901
};
903902

904903
self.visit_attrs_vis(&item.attrs, &item.vis);
905-
self.with_in_trait_impl(None, |this| {
906-
this.visibility_not_permitted(
907-
&item.vis,
908-
errors::VisibilityNotPermittedNote::IndividualImplItems,
909-
);
910-
if let &Safety::Unsafe(span) = safety {
911-
this.dcx().emit_err(errors::InherentImplCannotUnsafe {
912-
span: self_ty.span,
913-
annotation_span: span,
914-
annotation: "unsafe",
915-
self_ty: self_ty.span,
916-
});
917-
}
918-
if let &ImplPolarity::Negative(span) = polarity {
919-
this.dcx().emit_err(error(span, "negative", false));
920-
}
921-
if let &Defaultness::Default(def_span) = defaultness {
922-
this.dcx().emit_err(error(def_span, "`default`", true));
923-
}
924-
if let &Const::Yes(span) = constness {
925-
this.dcx().emit_err(error(span, "`const`", true));
926-
}
904+
self.visibility_not_permitted(
905+
&item.vis,
906+
errors::VisibilityNotPermittedNote::IndividualImplItems,
907+
);
908+
if let &Safety::Unsafe(span) = safety {
909+
self.dcx().emit_err(errors::InherentImplCannotUnsafe {
910+
span: self_ty.span,
911+
annotation_span: span,
912+
annotation: "unsafe",
913+
self_ty: self_ty.span,
914+
});
915+
}
916+
if let &ImplPolarity::Negative(span) = polarity {
917+
self.dcx().emit_err(error(span, "negative", false));
918+
}
919+
if let &Defaultness::Default(def_span) = defaultness {
920+
self.dcx().emit_err(error(def_span, "`default`", true));
921+
}
922+
if let &Const::Yes(span) = constness {
923+
self.dcx().emit_err(error(span, "`const`", true));
924+
}
927925

928-
this.with_tilde_const(
929-
Some(TildeConstReason::Impl { span: item.span }),
930-
|this| this.visit_generics(generics),
931-
);
932-
this.visit_ty(self_ty);
926+
self.with_tilde_const(Some(TildeConstReason::Impl { span: item.span }), |this| {
927+
this.visit_generics(generics)
928+
});
929+
self.visit_ty(self_ty);
930+
self.with_in_trait_impl(None, |this| {
933931
walk_list!(this, visit_assoc_item, items, AssocCtxt::Impl { of_trait: false });
934932
});
935933
}
@@ -976,34 +974,34 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
976974
self.visit_fn(kind, item.span, item.id);
977975
}
978976
ItemKind::ForeignMod(ForeignMod { extern_span, abi, safety, .. }) => {
979-
self.with_in_extern_mod(*safety, |this| {
980-
let old_item = mem::replace(&mut this.extern_mod_span, Some(item.span));
981-
this.visibility_not_permitted(
982-
&item.vis,
983-
errors::VisibilityNotPermittedNote::IndividualForeignItems,
984-
);
985-
986-
if &Safety::Default == safety {
987-
if item.span.at_least_rust_2024() {
988-
this.dcx().emit_err(errors::MissingUnsafeOnExtern { span: item.span });
989-
} else {
990-
this.lint_buffer.buffer_lint(
991-
MISSING_UNSAFE_ON_EXTERN,
992-
item.id,
993-
item.span,
994-
BuiltinLintDiag::MissingUnsafeOnExtern {
995-
suggestion: item.span.shrink_to_lo(),
996-
},
997-
);
998-
}
977+
let old_item = mem::replace(&mut self.extern_mod_span, Some(item.span));
978+
self.visibility_not_permitted(
979+
&item.vis,
980+
errors::VisibilityNotPermittedNote::IndividualForeignItems,
981+
);
982+
983+
if &Safety::Default == safety {
984+
if item.span.at_least_rust_2024() {
985+
self.dcx().emit_err(errors::MissingUnsafeOnExtern { span: item.span });
986+
} else {
987+
self.lint_buffer.buffer_lint(
988+
MISSING_UNSAFE_ON_EXTERN,
989+
item.id,
990+
item.span,
991+
BuiltinLintDiag::MissingUnsafeOnExtern {
992+
suggestion: item.span.shrink_to_lo(),
993+
},
994+
);
999995
}
996+
}
1000997

1001-
if abi.is_none() {
1002-
this.maybe_lint_missing_abi(*extern_span, item.id);
1003-
}
998+
if abi.is_none() {
999+
self.maybe_lint_missing_abi(*extern_span, item.id);
1000+
}
1001+
self.with_in_extern_mod(*safety, |this| {
10041002
visit::walk_item(this, item);
1005-
this.extern_mod_span = old_item;
10061003
});
1004+
self.extern_mod_span = old_item;
10071005
}
10081006
ItemKind::Enum(_, def, _) => {
10091007
for variant in &def.variants {
@@ -1024,24 +1022,23 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
10241022
self.visit_attrs_vis_ident(&item.attrs, &item.vis, ident);
10251023
let is_const_trait =
10261024
attr::find_by_name(&item.attrs, sym::const_trait).map(|attr| attr.span);
1027-
self.with_in_trait(item.span, is_const_trait, |this| {
1028-
if *is_auto == IsAuto::Yes {
1029-
// Auto traits cannot have generics, super traits nor contain items.
1030-
this.deny_generic_params(generics, ident.span);
1031-
this.deny_super_traits(bounds, ident.span);
1032-
this.deny_where_clause(&generics.where_clause, ident.span);
1033-
this.deny_items(items, ident.span);
1034-
}
1025+
if *is_auto == IsAuto::Yes {
1026+
// Auto traits cannot have generics, super traits nor contain items.
1027+
self.deny_generic_params(generics, ident.span);
1028+
self.deny_super_traits(bounds, ident.span);
1029+
self.deny_where_clause(&generics.where_clause, ident.span);
1030+
self.deny_items(items, ident.span);
1031+
}
10351032

1036-
// Equivalent of `visit::walk_item` for `ItemKind::Trait` that inserts a bound
1037-
// context for the supertraits.
1038-
let disallowed = is_const_trait
1039-
.is_none()
1040-
.then(|| TildeConstReason::Trait { span: item.span });
1041-
this.with_tilde_const(disallowed, |this| {
1042-
this.visit_generics(generics);
1043-
walk_list!(this, visit_param_bound, bounds, BoundKind::SuperTraits)
1044-
});
1033+
// Equivalent of `visit::walk_item` for `ItemKind::Trait` that inserts a bound
1034+
// context for the supertraits.
1035+
let disallowed =
1036+
is_const_trait.is_none().then(|| TildeConstReason::Trait { span: item.span });
1037+
self.with_tilde_const(disallowed, |this| {
1038+
this.visit_generics(generics);
1039+
walk_list!(this, visit_param_bound, bounds, BoundKind::SuperTraits)
1040+
});
1041+
self.with_in_trait(item.span, is_const_trait, |this| {
10451042
walk_list!(this, visit_assoc_item, items, AssocCtxt::Trait);
10461043
});
10471044
}

0 commit comments

Comments
 (0)