Skip to content

Refactor AST trait bound modifiers #119163

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Dec 22, 2023
Merged

Refactor AST trait bound modifiers #119163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 51 additions & 36 deletions compiler/rustc_ast/src/ast.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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),
}

Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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)]
Expand All @@ -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,
Expand All @@ -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.
Expand Down Expand Up @@ -3255,7 +3270,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);
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_ast_lowering/src/item.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1371,7 +1371,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);
Expand Down
53 changes: 27 additions & 26 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Negligible behavior change in the error path: We now lower_poly_trait_ref for ~const ! (master: TBM::MaybeConstNegative) instead of mapping it to None. We now only map ? & ~const ? to None which seems more logical. In any case, ~const ! gets rejected in AST validation, shouldn't have any user-facing effect.

| 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() {
Expand Down Expand Up @@ -2032,9 +2029,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))
Expand Down Expand Up @@ -2320,25 +2317,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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_passes/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
33 changes: 17 additions & 16 deletions compiler/rustc_ast_passes/src/ast_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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(
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_ast_passes/src/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_ast_pretty/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 12 additions & 18 deletions compiler/rustc_ast_pretty/src/pprust/state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down
Loading