Auto merge of rust-lang#119163 - fmease:refactor-ast-trait-bound-modifiers, r=compiler-errors

Refactor AST trait bound modifiers

Instead of having two types to represent trait bound modifiers in the parser / the AST (`parser::ty::BoundModifiers` & `ast::TraitBoundModifier`), only to map one to the other later, just use `parser::ty::BoundModifiers` (moved & renamed to `ast::TraitBoundModifiers`).

The struct type is more extensible and easier to deal with (see [here](https://github.com/rust-lang/rust/pull/119099/files#r1430749981) and [here](https://github.com/rust-lang/rust/pull/119099/files#r1430752116) for context) since it more closely models what it represents: A compound of two kinds of modifiers, constness and polarity. Modeling this as an enum (the now removed `ast::TraitBoundModifier`) meant one had to add a new variant per *combination* of modifier kind, which simply isn't scalable and which lead to a lot of explicit non-DRY matches.

NB: `hir::TraitBoundModifier` being an enum is fine since HIR doesn't need to worry representing invalid modifier kind combinations as those get rejected during AST validation thereby immensely cutting down the number of possibilities.

Author: bors

compiler/rustc_ast/src/ast.rs

@@ -286,41 +286,16 @@ impl ParenthesizedArgs {
 
 pub use crate::node_id::{NodeId, CRATE_NODE_ID, DUMMY_NODE_ID};
 
-/// A modifier on a bound, e.g., `?Trait` or `~const Trait`.
-///
-/// Negative bounds should also be handled here.
+/// Modifiers on a trait bound like `~const`, `?` and `!`.
 #[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug)]
-pub enum TraitBoundModifier {
-    /// No modifiers
-    None,
-
-    /// `!Trait`
-    Negative,
-
-    /// `?Trait`
-    Maybe,
-
-    /// `~const Trait`
-    MaybeConst(Span),
-
-    /// `~const !Trait`
-    //
-    // This parses but will be rejected during AST validation.
-    MaybeConstNegative,
-
-    /// `~const ?Trait`
-    //
-    // This parses but will be rejected during AST validation.
-    MaybeConstMaybe,
+pub struct TraitBoundModifiers {
+    pub constness: BoundConstness,
+    pub polarity: BoundPolarity,
 }
 
-impl TraitBoundModifier {
-    pub fn to_constness(self) -> Const {
-        match self {
-            Self::MaybeConst(span) => Const::Yes(span),
-            _ => Const::No,
-        }
-    }
+impl TraitBoundModifiers {
+    pub const NONE: Self =
+        Self { constness: BoundConstness::Never, polarity: BoundPolarity::Positive };
 }
 
 /// The AST represents all type param bounds as types.
@@ -329,7 +304,7 @@ impl TraitBoundModifier {
 /// detects `Copy`, `Send` and `Sync`.
 #[derive(Clone, Encodable, Decodable, Debug)]
 pub enum GenericBound {
-    Trait(PolyTraitRef, TraitBoundModifier),
+    Trait(PolyTraitRef, TraitBoundModifiers),
     Outlives(Lifetime),
 }
 
@@ -1193,7 +1168,7 @@ impl Expr {
         match &self.kind {
             ExprKind::Path(None, path) => Some(GenericBound::Trait(
                 PolyTraitRef::new(ThinVec::new(), path.clone(), self.span),
-                TraitBoundModifier::None,
+                TraitBoundModifiers::NONE,
             )),
             _ => None,
         }
@@ -2491,6 +2466,15 @@ pub enum Const {
     No,
 }
 
+impl From<BoundConstness> for Const {
+    fn from(constness: BoundConstness) -> Self {
+        match constness {
+            BoundConstness::Maybe(span) => Self::Yes(span),
+            BoundConstness::Never => Self::No,
+        }
+    }
+}
+
 /// Item defaultness.
 /// For details see the [RFC #2532](https://github.com/rust-lang/rfcs/pull/2532).
 #[derive(Copy, Clone, PartialEq, Encodable, Decodable, Debug, HashStable_Generic)]
@@ -2516,7 +2500,9 @@ impl fmt::Debug for ImplPolarity {
     }
 }
 
-#[derive(Copy, Clone, PartialEq, Encodable, Decodable, HashStable_Generic)]
+/// The polarity of a trait bound.
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug)]
+#[derive(HashStable_Generic)]
 pub enum BoundPolarity {
     /// `Type: Trait`
     Positive,
@@ -2526,6 +2512,35 @@ pub enum BoundPolarity {
     Maybe(Span),
 }
 
+impl BoundPolarity {
+    pub fn as_str(self) -> &'static str {
+        match self {
+            Self::Positive => "",
+            Self::Negative(_) => "!",
+            Self::Maybe(_) => "?",
+        }
+    }
+}
+
+/// The constness of a trait bound.
+#[derive(Copy, Clone, PartialEq, Eq, Encodable, Decodable, Debug)]
+#[derive(HashStable_Generic)]
+pub enum BoundConstness {
+    /// `Type: Trait`
+    Never,
+    /// `Type: ~const Trait`
+    Maybe(Span),
+}
+
+impl BoundConstness {
+    pub fn as_str(self) -> &'static str {
+        match self {
+            Self::Never => "",
+            Self::Maybe(_) => "~const",
+        }
+    }
+}
+
 #[derive(Clone, Encodable, Decodable, Debug)]
 pub enum FnRetTy {
     /// Returns type is not specified.
@@ -3259,7 +3274,7 @@ mod size_asserts {
     static_assert_size!(ForeignItem, 96);
     static_assert_size!(ForeignItemKind, 24);
     static_assert_size!(GenericArg, 24);
-    static_assert_size!(GenericBound, 64);
+    static_assert_size!(GenericBound, 72);
     static_assert_size!(Generics, 40);
     static_assert_size!(Impl, 136);
     static_assert_size!(Item, 136);

compiler/rustc_ast_lowering/src/item.rs

@@ -1372,7 +1372,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
             // need to compute this at all unless there is a Maybe bound.
             let mut is_param: Option<bool> = None;
             for bound in &bound_pred.bounds {
-                if !matches!(*bound, GenericBound::Trait(_, TraitBoundModifier::Maybe)) {
+                if !matches!(
+                    *bound,
+                    GenericBound::Trait(
+                        _,
+                        TraitBoundModifiers { polarity: BoundPolarity::Maybe(_), .. }
+                    )
+                ) {
                     continue;
                 }
                 let is_param = *is_param.get_or_insert_with(compute_is_param);

compiler/rustc_ast_lowering/src/lib.rs

@@ -1425,19 +1425,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
                         this.arena.alloc_from_iter(bounds.iter().filter_map(|bound| match bound {
                             GenericBound::Trait(
                                 ty,
-                                modifier @ (TraitBoundModifier::None
-                                | TraitBoundModifier::MaybeConst(_)
-                                | TraitBoundModifier::Negative),
-                            ) => {
-                                Some(this.lower_poly_trait_ref(ty, itctx, modifier.to_constness()))
-                            }
-                            // `~const ?Bound` will cause an error during AST validation
-                            // anyways, so treat it like `?Bound` as compilation proceeds.
+                                TraitBoundModifiers {
+                                    polarity: BoundPolarity::Positive | BoundPolarity::Negative(_),
+                                    constness,
+                                },
+                            ) => Some(this.lower_poly_trait_ref(ty, itctx, (*constness).into())),
+                            // We can safely ignore constness here, since AST validation
+                            // will take care of invalid modifier combinations.
                             GenericBound::Trait(
                                 _,
-                                TraitBoundModifier::Maybe
-                                | TraitBoundModifier::MaybeConstMaybe
-                                | TraitBoundModifier::MaybeConstNegative,
+                                TraitBoundModifiers { polarity: BoundPolarity::Maybe(_), .. },
                             ) => None,
                             GenericBound::Outlives(lifetime) => {
                                 if lifetime_bound.is_none() {
@@ -2028,9 +2025,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         itctx: &ImplTraitContext,
     ) -> hir::GenericBound<'hir> {
         match tpb {
-            GenericBound::Trait(p, modifier) => hir::GenericBound::Trait(
-                self.lower_poly_trait_ref(p, itctx, modifier.to_constness()),
-                self.lower_trait_bound_modifier(*modifier),
+            GenericBound::Trait(p, modifiers) => hir::GenericBound::Trait(
+                self.lower_poly_trait_ref(p, itctx, modifiers.constness.into()),
+                self.lower_trait_bound_modifiers(*modifiers),
             ),
             GenericBound::Outlives(lifetime) => {
                 hir::GenericBound::Outlives(self.lower_lifetime(lifetime))
@@ -2316,25 +2313,29 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
         }
     }
 
-    fn lower_trait_bound_modifier(&mut self, f: TraitBoundModifier) -> hir::TraitBoundModifier {
-        match f {
-            TraitBoundModifier::None => hir::TraitBoundModifier::None,
-            TraitBoundModifier::MaybeConst(_) => hir::TraitBoundModifier::MaybeConst,
-
-            TraitBoundModifier::Negative => {
+    fn lower_trait_bound_modifiers(
+        &mut self,
+        modifiers: TraitBoundModifiers,
+    ) -> hir::TraitBoundModifier {
+        match (modifiers.constness, modifiers.polarity) {
+            (BoundConstness::Never, BoundPolarity::Positive) => hir::TraitBoundModifier::None,
+            (BoundConstness::Never, BoundPolarity::Maybe(_)) => hir::TraitBoundModifier::Maybe,
+            (BoundConstness::Never, BoundPolarity::Negative(_)) => {
                 if self.tcx.features().negative_bounds {
                     hir::TraitBoundModifier::Negative
                 } else {
                     hir::TraitBoundModifier::None
                 }
             }
-
-            // `MaybeConstMaybe` will cause an error during AST validation, but we need to pick a
-            // placeholder for compilation to proceed.
-            TraitBoundModifier::MaybeConstMaybe | TraitBoundModifier::Maybe => {
-                hir::TraitBoundModifier::Maybe
+            (BoundConstness::Maybe(_), BoundPolarity::Positive) => {
+                hir::TraitBoundModifier::MaybeConst
+            }
+            // Invalid modifier combinations will cause an error during AST validation.
+            // Arbitrarily pick a placeholder for compilation to proceed.
+            (BoundConstness::Maybe(_), BoundPolarity::Maybe(_)) => hir::TraitBoundModifier::Maybe,
+            (BoundConstness::Maybe(_), BoundPolarity::Negative(_)) => {
+                hir::TraitBoundModifier::MaybeConst
             }
-            TraitBoundModifier::MaybeConstNegative => hir::TraitBoundModifier::MaybeConst,
         }
     }
 

compiler/rustc_ast_passes/messages.ftl

@@ -152,6 +152,8 @@ ast_passes_impl_trait_path = `impl Trait` is not allowed in path parameters
 ast_passes_incompatible_features = `{$f1}` and `{$f2}` are incompatible, using them at the same time is not allowed
     .help = remove one of these features
 
+ast_passes_incompatible_trait_bound_modifiers = `{$left}` and `{$right}` are mutually exclusive
+
 ast_passes_inherent_cannot_be = inherent impls cannot be {$annotation}
     .because = {$annotation} because of this
     .type = inherent impl for this type
@@ -195,8 +197,6 @@ ast_passes_nomangle_ascii = `#[no_mangle]` requires ASCII identifier
 ast_passes_obsolete_auto = `impl Trait for .. {"{}"}` is an obsolete syntax
     .help = use `auto trait Trait {"{}"}` instead
 
-ast_passes_optional_const_exclusive = `~const` and `{$modifier}` are mutually exclusive
-
 ast_passes_optional_trait_object = `?Trait` is not permitted in trait object types
 
 ast_passes_optional_trait_supertrait = `?Trait` is not permitted in supertraits

compiler/rustc_ast_passes/src/ast_validation.rs

@@ -1196,18 +1196,18 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
     }
 
     fn visit_param_bound(&mut self, bound: &'a GenericBound, ctxt: BoundKind) {
-        if let GenericBound::Trait(poly, modify) = bound {
-            match (ctxt, modify) {
-                (BoundKind::SuperTraits, TraitBoundModifier::Maybe) => {
+        if let GenericBound::Trait(poly, modifiers) = bound {
+            match (ctxt, modifiers.constness, modifiers.polarity) {
+                (BoundKind::SuperTraits, BoundConstness::Never, BoundPolarity::Maybe(_)) => {
                     self.dcx().emit_err(errors::OptionalTraitSupertrait {
                         span: poly.span,
                         path_str: pprust::path_to_string(&poly.trait_ref.path),
                     });
                 }
-                (BoundKind::TraitObject, TraitBoundModifier::Maybe) => {
+                (BoundKind::TraitObject, BoundConstness::Never, BoundPolarity::Maybe(_)) => {
                     self.dcx().emit_err(errors::OptionalTraitObject { span: poly.span });
                 }
-                (_, &TraitBoundModifier::MaybeConst(span))
+                (_, BoundConstness::Maybe(span), BoundPolarity::Positive)
                     if let Some(reason) = &self.disallow_tilde_const =>
                 {
                     let reason = match reason {
@@ -1235,24 +1235,24 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
                     };
                     self.dcx().emit_err(errors::TildeConstDisallowed { span, reason });
                 }
-                (_, TraitBoundModifier::MaybeConstMaybe) => {
-                    self.dcx().emit_err(errors::OptionalConstExclusive {
+                (
+                    _,
+                    BoundConstness::Maybe(_),
+                    BoundPolarity::Maybe(_) | BoundPolarity::Negative(_),
+                ) => {
+                    self.dcx().emit_err(errors::IncompatibleTraitBoundModifiers {
                         span: bound.span(),
-                        modifier: "?",
-                    });
-                }
-                (_, TraitBoundModifier::MaybeConstNegative) => {
-                    self.dcx().emit_err(errors::OptionalConstExclusive {
-                        span: bound.span(),
-                        modifier: "!",
+                        left: modifiers.constness.as_str(),
+                        right: modifiers.polarity.as_str(),
                     });
                 }
                 _ => {}
             }
         }
 
         // Negative trait bounds are not allowed to have associated constraints
-        if let GenericBound::Trait(trait_ref, TraitBoundModifier::Negative) = bound
+        if let GenericBound::Trait(trait_ref, modifiers) = bound
+            && let BoundPolarity::Negative(_) = modifiers.polarity
             && let Some(segment) = trait_ref.trait_ref.path.segments.last()
             && let Some(ast::GenericArgs::AngleBracketed(args)) = segment.args.as_deref()
         {
@@ -1494,7 +1494,8 @@ fn deny_equality_constraints(
             for param in &generics.params {
                 if param.ident == potential_param.ident {
                     for bound in &param.bounds {
-                        if let ast::GenericBound::Trait(trait_ref, TraitBoundModifier::None) = bound
+                        if let ast::GenericBound::Trait(trait_ref, TraitBoundModifiers::NONE) =
+                            bound
                         {
                             if let [trait_segment] = &trait_ref.trait_ref.path.segments[..] {
                                 let assoc = pprust::path_to_string(&ast::Path::from_ident(

compiler/rustc_ast_passes/src/errors.rs

@@ -580,11 +580,12 @@ pub enum TildeConstReason {
 }
 
 #[derive(Diagnostic)]
-#[diag(ast_passes_optional_const_exclusive)]
-pub struct OptionalConstExclusive {
+#[diag(ast_passes_incompatible_trait_bound_modifiers)]
+pub struct IncompatibleTraitBoundModifiers {
     #[primary_span]
     pub span: Span,
-    pub modifier: &'static str,
+    pub left: &'static str,
+    pub right: &'static str,
 }
 
 #[derive(Diagnostic)]

compiler/rustc_ast_pretty/src/lib.rs

@@ -4,6 +4,7 @@
 #![deny(rustc::untranslatable_diagnostic)]
 #![deny(rustc::diagnostic_outside_of_impl)]
 #![feature(box_patterns)]
+#![feature(let_chains)]
 #![recursion_limit = "256"]
 
 mod helpers;

compiler/rustc_ast_pretty/src/pprust/state.rs

@@ -17,7 +17,7 @@ use rustc_ast::util::comments::{gather_comments, Comment, CommentStyle};
 use rustc_ast::util::parser;
 use rustc_ast::{self as ast, AttrArgs, AttrArgsEq, BlockCheckMode, PatKind};
 use rustc_ast::{attr, BindingAnnotation, ByRef, DelimArgs, RangeEnd, RangeSyntax, Term};
-use rustc_ast::{GenericArg, GenericBound, SelfKind, TraitBoundModifier};
+use rustc_ast::{GenericArg, GenericBound, SelfKind};
 use rustc_ast::{InlineAsmOperand, InlineAsmRegOrRegClass};
 use rustc_ast::{InlineAsmOptions, InlineAsmTemplatePiece};
 use rustc_span::edition::Edition;
@@ -1559,26 +1559,20 @@ impl<'a> State<'a> {
 
             match bound {
                 GenericBound::Trait(tref, modifier) => {
-                    match modifier {
-                        TraitBoundModifier::None => {}
-                        TraitBoundModifier::Negative => {
-                            self.word("!");
+                    match modifier.constness {
+                        ast::BoundConstness::Never => {}
+                        ast::BoundConstness::Maybe(_) => {
+                            self.word_space(modifier.constness.as_str());
                         }
-                        TraitBoundModifier::Maybe => {
-                            self.word("?");
-                        }
-                        TraitBoundModifier::MaybeConst(_) => {
-                            self.word_space("~const");
-                        }
-                        TraitBoundModifier::MaybeConstNegative => {
-                            self.word_space("~const");
-                            self.word("!");
-                        }
-                        TraitBoundModifier::MaybeConstMaybe => {
-                            self.word_space("~const");
-                            self.word("?");
+                    }
+
+                    match modifier.polarity {
+                        ast::BoundPolarity::Positive => {}
+                        ast::BoundPolarity::Negative(_) | ast::BoundPolarity::Maybe(_) => {
+                            self.word(modifier.polarity.as_str());
                         }
                     }
+
                     self.print_poly_trait_ref(tref);
                 }
                 GenericBound::Outlives(lt) => self.print_lifetime(*lt),