Skip to content

Commit

Permalink
Unrolled build for rust-lang#127054
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#127054 - compiler-errors:bound-ordering, r=fmease

Reorder trait bound modifiers *after* `for<...>` binder in trait bounds

This PR suggests changing the grammar of trait bounds from:

```
[CONSTNESS] [ASYNCNESS] [?] [BINDER] [TRAIT_PATH]

const async ? for<'a> Sized
```

to

```
([BINDER] [CONSTNESS] [ASYNCNESS] | [?]) [TRAIT_PATH]
```

i.e., either

```
? Sized
```

or

```
for<'a> const async Sized
```

(but not both)

### Why?

I think it's strange that the binder applies "more tightly" than the `?` trait polarity. This becomes even weirder when considering that we (or at least, I) want to have `async` trait bounds expressed like:

```
where T: for<'a> async Fn(&'a ()) -> i32,
```

and not:

```
where T: async for<'a> Fn(&'a ()) -> i32,
```

### Fallout

No crates on crater use this syntax, presumably because it's literally useless. This will require modifying the reference grammar, though.

### Alternatives

If this is not desirable, then we can alternatively keep parsing `for<'a>` after the `?` but deprecate it with either an FCW (or an immediate hard error), and begin parsing `for<'a>` *before* the `?`.
  • Loading branch information
rust-timer authored Jul 25, 2024
2 parents e7d66ea + 7da751a commit 0d5c969
Show file tree
Hide file tree
Showing 20 changed files with 201 additions and 68 deletions.
2 changes: 0 additions & 2 deletions compiler/rustc_ast_passes/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ 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
11 changes: 0 additions & 11 deletions compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1366,17 +1366,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
{
self.dcx().emit_err(errors::TildeConstDisallowed { span, reason });
}
(
_,
BoundConstness::Always(_) | BoundConstness::Maybe(_),
BoundPolarity::Negative(_) | BoundPolarity::Maybe(_),
) => {
self.dcx().emit_err(errors::IncompatibleTraitBoundModifiers {
span: bound.span(),
left: modifiers.constness.as_str(),
right: modifiers.polarity.as_str(),
});
}
_ => {}
}

Expand Down
9 changes: 0 additions & 9 deletions compiler/rustc_ast_passes/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,15 +656,6 @@ pub enum TildeConstReason {
Item,
}

#[derive(Diagnostic)]
#[diag(ast_passes_incompatible_trait_bound_modifiers)]
pub struct IncompatibleTraitBoundModifiers {
#[primary_span]
pub span: Span,
pub left: &'static str,
pub right: &'static str,
}

#[derive(Diagnostic)]
#[diag(ast_passes_const_and_async)]
pub struct ConstAndAsync {
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_parse/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ parse_bare_cr = {$double_quotes ->
parse_bare_cr_in_raw_string = bare CR not allowed in raw string
parse_binder_and_polarity = `for<...>` binder not allowed with `{$polarity}` trait polarity modifier
.label = there is not a well-defined meaning for a higher-ranked `{$polarity}` trait
parse_binder_before_modifiers = `for<...>` binder should be placed before trait bound modifiers
.label = place the `for<...>` binder before any modifiers
parse_bounds_not_allowed_on_trait_aliases = bounds are not allowed on trait aliases
parse_box_not_pat = expected pattern, found {$descr}
Expand Down Expand Up @@ -577,6 +583,9 @@ parse_missing_trait_in_trait_impl = missing trait in a trait impl
parse_modifier_lifetime = `{$modifier}` may only modify trait bounds, not lifetime bounds
.suggestion = remove the `{$modifier}`
parse_modifiers_and_polarity = `{$modifiers_concatenated}` trait not allowed with `{$polarity}` trait polarity modifier
.label = there is not a well-defined meaning for a `{$modifiers_concatenated} {$polarity}` trait
parse_more_than_one_char = character literal may only contain one codepoint
.followed_by = this `{$chr}` is followed by the combining {$len ->
[one] mark
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_parse/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3212,3 +3212,33 @@ pub struct UnsafeAttrOutsideUnsafeSuggestion {
#[suggestion_part(code = ")")]
pub right: Span,
}

#[derive(Diagnostic)]
#[diag(parse_binder_before_modifiers)]
pub struct BinderBeforeModifiers {
#[primary_span]
pub binder_span: Span,
#[label]
pub modifiers_span: Span,
}

#[derive(Diagnostic)]
#[diag(parse_binder_and_polarity)]
pub struct BinderAndPolarity {
#[primary_span]
pub polarity_span: Span,
#[label]
pub binder_span: Span,
pub polarity: &'static str,
}

#[derive(Diagnostic)]
#[diag(parse_modifiers_and_polarity)]
pub struct PolarityAndModifiers {
#[primary_span]
pub polarity_span: Span,
#[label]
pub modifiers_span: Span,
pub polarity: &'static str,
pub modifiers_concatenated: String,
}
60 changes: 57 additions & 3 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,9 +935,14 @@ impl<'a> Parser<'a> {
/// If no modifiers are present, this does not consume any tokens.
///
/// ```ebnf
/// TRAIT_BOUND_MODIFIERS = [["~"] "const"] ["async"] ["?" | "!"]
/// CONSTNESS = [["~"] "const"]
/// ASYNCNESS = ["async"]
/// POLARITY = ["?" | "!"]
/// ```
///
/// See `parse_generic_ty_bound` for the complete grammar of trait bound modifiers.
fn parse_trait_bound_modifiers(&mut self) -> PResult<'a, TraitBoundModifiers> {
let modifier_lo = self.token.span;
let constness = if self.eat(&token::Tilde) {
let tilde = self.prev_token.span;
self.expect_keyword(kw::Const)?;
Expand Down Expand Up @@ -970,6 +975,7 @@ impl<'a> Parser<'a> {
} else {
BoundAsyncness::Normal
};
let modifier_hi = self.prev_token.span;

let polarity = if self.eat(&token::Question) {
BoundPolarity::Maybe(self.prev_token.span)
Expand All @@ -980,13 +986,40 @@ impl<'a> Parser<'a> {
BoundPolarity::Positive
};

// Enforce the mutual-exclusivity of `const`/`async` and `?`/`!`.
match polarity {
BoundPolarity::Positive => {
// All trait bound modifiers allowed to combine with positive polarity
}
BoundPolarity::Maybe(polarity_span) | BoundPolarity::Negative(polarity_span) => {
match (asyncness, constness) {
(BoundAsyncness::Normal, BoundConstness::Never) => {
// Ok, no modifiers.
}
(_, _) => {
let constness = constness.as_str();
let asyncness = asyncness.as_str();
let glue =
if !constness.is_empty() && !asyncness.is_empty() { " " } else { "" };
let modifiers_concatenated = format!("{constness}{glue}{asyncness}");
self.dcx().emit_err(errors::PolarityAndModifiers {
polarity_span,
polarity: polarity.as_str(),
modifiers_span: modifier_lo.to(modifier_hi),
modifiers_concatenated,
});
}
}
}
}

Ok(TraitBoundModifiers { constness, asyncness, polarity })
}

/// Parses a type bound according to:
/// ```ebnf
/// TY_BOUND = TY_BOUND_NOPAREN | (TY_BOUND_NOPAREN)
/// TY_BOUND_NOPAREN = [TRAIT_BOUND_MODIFIERS] [for<LT_PARAM_DEFS>] SIMPLE_PATH
/// TY_BOUND_NOPAREN = [for<GENERIC_PARAMS> CONSTNESS ASYNCNESS | POLARITY] SIMPLE_PATH
/// ```
///
/// For example, this grammar accepts `for<'a: 'b> ~const ?m::Trait<'a>`.
Expand All @@ -996,16 +1029,37 @@ impl<'a> Parser<'a> {
has_parens: bool,
leading_token: &Token,
) -> PResult<'a, GenericBound> {
let modifiers = self.parse_trait_bound_modifiers()?;
let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;

let modifiers_lo = self.token.span;
let modifiers = self.parse_trait_bound_modifiers()?;
let modifiers_span = modifiers_lo.to(self.prev_token.span);

if let Some(binder_span) = binder_span {
match modifiers.polarity {
BoundPolarity::Negative(polarity_span) | BoundPolarity::Maybe(polarity_span) => {
self.dcx().emit_err(errors::BinderAndPolarity {
binder_span,
polarity_span,
polarity: modifiers.polarity.as_str(),
});
}
BoundPolarity::Positive => {}
}
}

// Recover erroneous lifetime bound with modifiers or binder.
// e.g. `T: for<'a> 'a` or `T: ~const 'a`.
if self.token.is_lifetime() {
let _: ErrorGuaranteed = self.error_lt_bound_with_modifiers(modifiers, binder_span);
return self.parse_generic_lt_bound(lo, has_parens);
}

if let (more_lifetime_defs, Some(binder_span)) = self.parse_late_bound_lifetime_defs()? {
lifetime_defs.extend(more_lifetime_defs);
self.dcx().emit_err(errors::BinderBeforeModifiers { binder_span, modifiers_span });
}

let mut path = if self.token.is_keyword(kw::Fn)
&& self.look_ahead(1, |tok| tok.kind == TokenKind::OpenDelim(Delimiter::Parenthesis))
&& let Some(path) = self.recover_path_from_fn()
Expand Down
2 changes: 0 additions & 2 deletions src/tools/rustfmt/tests/source/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,6 @@ trait T: ~ const Super {}

const fn not_quite_const<S: ~ const T>() -> i32 { <S as T>::CONST }

struct S<T:~ const ? Sized>(std::marker::PhantomData<T>);

impl ~ const T {}

fn apit(_: impl ~ const T) {}
Expand Down
6 changes: 0 additions & 6 deletions src/tools/rustfmt/tests/target/negative-bounds.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,3 @@ where
i32: !Copy,
{
}

fn maybe_const_negative()
where
i32: ~const !Copy,
{
}
2 changes: 0 additions & 2 deletions src/tools/rustfmt/tests/target/type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,6 @@ const fn not_quite_const<S: ~const T>() -> i32 {
<S as T>::CONST
}

struct S<T: ~const ?Sized>(std::marker::PhantomData<T>);

impl ~const T {}

fn apit(_: impl ~const T) {}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use std::path::{Path, PathBuf};
const ENTRY_LIMIT: u32 = 901;
// FIXME: The following limits should be reduced eventually.

const ISSUES_ENTRY_LIMIT: u32 = 1672;
const ISSUES_ENTRY_LIMIT: u32 = 1673;

const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
"rs", // test source files
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/async-await/async-fn/higher-ranked-async-fn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ async fn f(arg: &i32) {}

async fn func<F>(f: F)
where
F: async for<'a> Fn(&'a i32),
F: for<'a> async Fn(&'a i32),
{
let x: i32 = 0;
f(&x).await;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/impl-trait/normalize-tait-in-const.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ mod foo {
}
use foo::*;

const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
fun(filter_positive());
}

Expand Down
8 changes: 4 additions & 4 deletions tests/ui/impl-trait/normalize-tait-in-const.stderr
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
error: `~const` can only be applied to `#[const_trait]` traits
--> $DIR/normalize-tait-in-const.rs:27:42
|
LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
| ^^^^^^^^^^^^^^^^^

error: `~const` can only be applied to `#[const_trait]` traits
--> $DIR/normalize-tait-in-const.rs:27:69
|
LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
| ^^^^^^^^

error[E0015]: cannot call non-const closure in constant functions
Expand All @@ -19,7 +19,7 @@ LL | fun(filter_positive());
= note: calls in constant functions are limited to constant functions, tuple structs and tuple variants
help: consider further restricting this bound
|
LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct + ~const Fn(&foo::Alias<'_>)>(fun: F) {
LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct + ~const Fn(&foo::Alias<'_>)>(fun: F) {
| ++++++++++++++++++++++++++++
help: add `#![feature(effects)]` to the crate attributes to enable
|
Expand All @@ -29,7 +29,7 @@ LL + #![feature(effects)]
error[E0493]: destructor of `F` cannot be evaluated at compile-time
--> $DIR/normalize-tait-in-const.rs:27:79
|
LL | const fn with_positive<F: ~const for<'a> Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
LL | const fn with_positive<F: for<'a> ~const Fn(&'a Alias<'a>) + ~const Destruct>(fun: F) {
| ^^^ the destructor for this type cannot be evaluated in constant functions
LL | fun(filter_positive());
LL | }
Expand Down
3 changes: 1 addition & 2 deletions tests/ui/issues/issue-39089.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
//@ check-pass
#![allow(dead_code)]
fn f<T: ?for<'a> Sized>() {}
//~^ ERROR `for<...>` binder should be placed before trait bound modifiers

fn main() {}
10 changes: 10 additions & 0 deletions tests/ui/issues/issue-39089.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: `for<...>` binder should be placed before trait bound modifiers
--> $DIR/issue-39089.rs:1:13
|
LL | fn f<T: ?for<'a> Sized>() {}
| - ^^^^
| |
| place the `for<...>` binder before any modifiers

error: aborting due to 1 previous error

15 changes: 13 additions & 2 deletions tests/ui/parser/bounds-type.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,30 @@
//@ compile-flags: -Z parse-only
//@ edition: 2021

struct S<
T: 'a + Tr, // OK
T: Tr + 'a, // OK
T: 'a, // OK
T:, // OK
T: ?for<'a> Trait, // OK
T: for<'a> ?Trait, //~ ERROR `for<...>` binder not allowed with `?` trait polarity modifier
T: Tr +, // OK
T: ?'a, //~ ERROR `?` may only modify trait bounds, not lifetime bounds

T: ~const Tr, // OK
T: ~const ?Tr, // OK
T: ~const ?Tr, //~ ERROR `~const` trait not allowed with `?` trait polarity modifier
T: ~const Tr + 'a, // OK
T: ~const 'a, //~ ERROR `~const` may only modify trait bounds, not lifetime bounds
T: const 'a, //~ ERROR `const` may only modify trait bounds, not lifetime bounds

T: async Tr, // OK
T: async ?Tr, //~ ERROR `async` trait not allowed with `?` trait polarity modifier
T: async Tr + 'a, // OK
T: async 'a, //~ ERROR `async` may only modify trait bounds, not lifetime bounds

T: const async Tr, // OK
T: const async ?Tr, //~ ERROR `const async` trait not allowed with `?` trait polarity modifier
T: const async Tr + 'a, // OK
T: const async 'a, //~ ERROR `const` may only modify trait bounds, not lifetime bounds
>;

fn main() {}
Loading

0 comments on commit 0d5c969

Please sign in to comment.