From 9a4c1058fac8cbbf771740928ec9a954b451c653 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 8 Jul 2024 09:48:12 +0000 Subject: [PATCH 1/3] Just store a span instead of the whole item --- compiler/rustc_ast_passes/src/ast_validation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index dd0d904c52c..507329cc899 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -81,7 +81,7 @@ struct AstValidator<'a> { features: &'a Features, /// The span of the `extern` in an `extern { ... }` block, if any. - extern_mod: Option<&'a Item>, + extern_mod: Option, outer_trait_or_trait_impl: Option>, @@ -579,7 +579,7 @@ fn check_foreign_fn_bodyless(&self, ident: Ident, body: Option<&Block>) { } fn current_extern_span(&self) -> Span { - self.session.source_map().guess_head_span(self.extern_mod.unwrap().span) + self.session.source_map().guess_head_span(self.extern_mod.unwrap()) } /// An `fn` in `extern { ... }` cannot have qualifiers, e.g. `async fn`. @@ -1080,7 +1080,7 @@ fn visit_item(&mut self, item: &'a Item) { } ItemKind::ForeignMod(ForeignMod { abi, safety, .. }) => { self.with_in_extern_mod(*safety, |this| { - let old_item = mem::replace(&mut this.extern_mod, Some(item)); + let old_item = mem::replace(&mut this.extern_mod, Some(item.span)); this.visibility_not_permitted( &item.vis, errors::VisibilityNotPermittedNote::IndividualForeignItems, From b879e29864c30d2bb2be2fffb910d66f0f4e9773 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jul 2024 08:54:11 +0000 Subject: [PATCH 2/3] Remove a needless borrow --- compiler/rustc_ast_passes/src/ast_validation.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 507329cc899..45c6211598b 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -61,12 +61,12 @@ enum DisallowTildeConstContext<'a> { Item, } -enum TraitOrTraitImpl<'a> { +enum TraitOrTraitImpl { Trait { span: Span, constness: Option }, - TraitImpl { constness: Const, polarity: ImplPolarity, trait_ref: &'a TraitRef }, + TraitImpl { constness: Const, polarity: ImplPolarity, trait_ref: Span }, } -impl<'a> TraitOrTraitImpl<'a> { +impl TraitOrTraitImpl { fn constness(&self) -> Option { match self { Self::Trait { constness: Some(span), .. } @@ -83,7 +83,7 @@ struct AstValidator<'a> { /// The span of the `extern` in an `extern { ... }` block, if any. extern_mod: Option, - outer_trait_or_trait_impl: Option>, + outer_trait_or_trait_impl: Option, has_proc_macro_decls: bool, @@ -115,7 +115,7 @@ fn with_in_trait_impl( trait_.map(|(constness, polarity, trait_ref)| TraitOrTraitImpl::TraitImpl { constness, polarity, - trait_ref, + trait_ref: trait_ref.path.span, }), ); f(self); @@ -354,7 +354,7 @@ fn deny_unnamed_field(&self, field: &FieldDef) { } } - fn check_trait_fn_not_const(&self, constness: Const, parent: &TraitOrTraitImpl<'a>) { + fn check_trait_fn_not_const(&self, constness: Const, parent: &TraitOrTraitImpl) { let Const::Yes(span) = constness else { return; }; @@ -367,7 +367,7 @@ fn check_trait_fn_not_const(&self, constness: Const, parent: &TraitOrTraitImpl<' .. } = parent { - Some(trait_ref.path.span.shrink_to_lo()) + Some(trait_ref.shrink_to_lo()) } else { None }; From d9f959292464c7bd68672801a42113e5b6d262b8 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Tue, 9 Jul 2024 09:54:19 +0000 Subject: [PATCH 3/3] Remove a boilerplaty abstraction --- .../rustc_ast_passes/src/ast_validation.rs | 76 ++++--------------- compiler/rustc_ast_passes/src/errors.rs | 2 +- 2 files changed, 17 insertions(+), 61 deletions(-) diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 45c6211598b..e6cc2e4069b 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -38,7 +38,7 @@ use std::ops::{Deref, DerefMut}; use thin_vec::thin_vec; -use crate::errors; +use crate::errors::{self, TildeConstReason}; /// Is `self` allowed semantically as the first parameter in an `FnDecl`? enum SelfSemantic { @@ -46,21 +46,6 @@ enum SelfSemantic { No, } -/// What is the context that prevents using `~const`? -// FIXME(effects): Consider getting rid of this in favor of `errors::TildeConstReason`, they're -// almost identical. This gets rid of an abstraction layer which might be considered bad. -enum DisallowTildeConstContext<'a> { - TraitObject, - Fn(FnKind<'a>), - Trait(Span), - TraitImpl(Span), - Impl(Span), - TraitAssocTy(Span), - TraitImplAssocTy(Span), - InherentAssocTy(Span), - Item, -} - enum TraitOrTraitImpl { Trait { span: Span, constness: Option }, TraitImpl { constness: Const, polarity: ImplPolarity, trait_ref: Span }, @@ -92,7 +77,7 @@ struct AstValidator<'a> { /// e.g., `impl Iterator`. outer_impl_trait: Option, - disallow_tilde_const: Option>, + disallow_tilde_const: Option, /// Used to ban `impl Trait` in path projections like `::Item` /// or `Foo::Bar` @@ -145,7 +130,7 @@ fn with_banned_impl_trait(&mut self, f: impl FnOnce(&mut Self)) { fn with_tilde_const( &mut self, - disallowed: Option>, + disallowed: Option, f: impl FnOnce(&mut Self), ) { let old = mem::replace(&mut self.disallow_tilde_const, disallowed); @@ -224,7 +209,7 @@ fn walk_ty(&mut self, t: &'a Ty) { } } TyKind::TraitObject(..) => self - .with_tilde_const(Some(DisallowTildeConstContext::TraitObject), |this| { + .with_tilde_const(Some(TildeConstReason::TraitObject), |this| { visit::walk_ty(this, t) }), TyKind::Path(qself, path) => { @@ -980,7 +965,7 @@ fn visit_item(&mut self, item: &'a Item) { this.visit_vis(&item.vis); this.visit_ident(item.ident); let disallowed = matches!(constness, Const::No) - .then(|| DisallowTildeConstContext::TraitImpl(item.span)); + .then(|| TildeConstReason::TraitImpl { span: item.span }); this.with_tilde_const(disallowed, |this| this.visit_generics(generics)); this.visit_trait_ref(t); this.visit_ty(self_ty); @@ -1035,7 +1020,7 @@ fn visit_item(&mut self, item: &'a Item) { this.visit_vis(&item.vis); this.visit_ident(item.ident); this.with_tilde_const( - Some(DisallowTildeConstContext::Impl(item.span)), + Some(TildeConstReason::Impl { span: item.span }), |this| this.visit_generics(generics), ); this.visit_ty(self_ty); @@ -1154,7 +1139,7 @@ fn visit_item(&mut self, item: &'a Item) { this.visit_ident(item.ident); let disallowed = is_const_trait .is_none() - .then(|| DisallowTildeConstContext::Trait(item.span)); + .then(|| TildeConstReason::Trait { span: item.span }); this.with_tilde_const(disallowed, |this| { this.visit_generics(generics); walk_list!(this, visit_param_bound, bounds, BoundKind::SuperTraits) @@ -1399,40 +1384,8 @@ fn visit_param_bound(&mut self, bound: &'a GenericBound, ctxt: BoundKind) { self.dcx().emit_err(errors::ConstBoundTraitObject { span: trait_ref.span }); } (_, BoundConstness::Maybe(span), BoundPolarity::Positive) - if let Some(reason) = &self.disallow_tilde_const => + if let Some(reason) = self.disallow_tilde_const => { - let reason = match reason { - DisallowTildeConstContext::Fn(FnKind::Closure(..)) => { - errors::TildeConstReason::Closure - } - DisallowTildeConstContext::Fn(FnKind::Fn(_, ident, ..)) => { - errors::TildeConstReason::Function { ident: ident.span } - } - &DisallowTildeConstContext::Trait(span) => { - errors::TildeConstReason::Trait { span } - } - &DisallowTildeConstContext::TraitImpl(span) => { - errors::TildeConstReason::TraitImpl { span } - } - &DisallowTildeConstContext::Impl(span) => { - // FIXME(effects): Consider providing a help message or even a structured - // suggestion for moving such bounds to the assoc const fns if available. - errors::TildeConstReason::Impl { span } - } - &DisallowTildeConstContext::TraitAssocTy(span) => { - errors::TildeConstReason::TraitAssocTy { span } - } - &DisallowTildeConstContext::TraitImplAssocTy(span) => { - errors::TildeConstReason::TraitImplAssocTy { span } - } - &DisallowTildeConstContext::InherentAssocTy(span) => { - errors::TildeConstReason::InherentAssocTy { span } - } - DisallowTildeConstContext::TraitObject => { - errors::TildeConstReason::TraitObject - } - DisallowTildeConstContext::Item => errors::TildeConstReason::Item, - }; self.dcx().emit_err(errors::TildeConstDisallowed { span, reason }); } ( @@ -1569,7 +1522,10 @@ fn visit_fn(&mut self, fk: FnKind<'a>, span: Span, id: NodeId) { .and_then(TraitOrTraitImpl::constness) .is_some(); - let disallowed = (!tilde_const_allowed).then(|| DisallowTildeConstContext::Fn(fk)); + let disallowed = (!tilde_const_allowed).then(|| match fk { + FnKind::Fn(_, ident, _, _, _, _) => TildeConstReason::Function { ident: ident.span }, + FnKind::Closure(_, _, _) => TildeConstReason::Closure, + }); self.with_tilde_const(disallowed, |this| visit::walk_fn(this, fk)); } @@ -1664,12 +1620,12 @@ fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) { AssocItemKind::Type(_) => { let disallowed = (!parent_is_const).then(|| match self.outer_trait_or_trait_impl { Some(TraitOrTraitImpl::Trait { .. }) => { - DisallowTildeConstContext::TraitAssocTy(item.span) + TildeConstReason::TraitAssocTy { span: item.span } } Some(TraitOrTraitImpl::TraitImpl { .. }) => { - DisallowTildeConstContext::TraitImplAssocTy(item.span) + TildeConstReason::TraitImplAssocTy { span: item.span } } - None => DisallowTildeConstContext::InherentAssocTy(item.span), + None => TildeConstReason::InherentAssocTy { span: item.span }, }); self.with_tilde_const(disallowed, |this| { this.with_in_trait_impl(None, |this| visit::walk_assoc_item(this, item, ctxt)) @@ -1852,7 +1808,7 @@ pub fn check_crate( outer_trait_or_trait_impl: None, has_proc_macro_decls: false, outer_impl_trait: None, - disallow_tilde_const: Some(DisallowTildeConstContext::Item), + disallow_tilde_const: Some(TildeConstReason::Item), is_impl_trait_banned: false, extern_mod_safety: None, lint_buffer: lints, diff --git a/compiler/rustc_ast_passes/src/errors.rs b/compiler/rustc_ast_passes/src/errors.rs index bfb90476450..2c18b47f0f7 100644 --- a/compiler/rustc_ast_passes/src/errors.rs +++ b/compiler/rustc_ast_passes/src/errors.rs @@ -612,7 +612,7 @@ pub struct TildeConstDisallowed { pub reason: TildeConstReason, } -#[derive(Subdiagnostic)] +#[derive(Subdiagnostic, Copy, Clone)] pub enum TildeConstReason { #[note(ast_passes_closure)] Closure,