Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix split index calculation in check_type_alias_where_clause_location #138037

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
@@ -3429,7 +3429,9 @@ pub struct TyAliasWhereClauses {
/// The index in `TyAlias.generics.where_clause.predicates` that would split
/// into predicates from the where clause before the equals sign and the ones
/// from the where clause after the equals sign.
pub split: usize,
pub before_count: usize,
/// The count of the clauses that appear before the equals sign and have attrbutes.
pub before_with_attr_count: usize,
}

#[derive(Clone, Encodable, Decodable, Debug)]
2 changes: 1 addition & 1 deletion compiler/rustc_ast/src/mut_visit.rs
Original file line number Diff line number Diff line change
@@ -1091,7 +1091,7 @@ fn walk_generics<T: MutVisitor>(vis: &mut T, generics: &mut Generics) {
}

fn walk_ty_alias_where_clauses<T: MutVisitor>(vis: &mut T, tawcs: &mut TyAliasWhereClauses) {
let TyAliasWhereClauses { before, after, split: _ } = tawcs;
let TyAliasWhereClauses { before, after, before_count: _, before_with_attr_count: _ } = tawcs;
let TyAliasWhereClause { has_where_token: _, span: span_before } = before;
let TyAliasWhereClause { has_where_token: _, span: span_after } = after;
vis.visit_span(span_before);
4 changes: 3 additions & 1 deletion compiler/rustc_ast_passes/src/ast_validation.rs
Original file line number Diff line number Diff line change
@@ -136,8 +136,10 @@ impl<'a> AstValidator<'a> {
return Ok(());
}

let split =
ty_alias.where_clauses.before_count - ty_alias.where_clauses.before_with_attr_count;
let (before_predicates, after_predicates) =
ty_alias.generics.where_clause.predicates.split_at(ty_alias.where_clauses.split);
ty_alias.generics.where_clause.predicates.split_at(split);
let span = ty_alias.where_clauses.before.span;

let sugg = if !before_predicates.is_empty() || !ty_alias.where_clauses.after.has_where_token
2 changes: 1 addition & 1 deletion compiler/rustc_ast_pretty/src/pprust/state/item.rs
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ impl<'a> State<'a> {
defaultness: ast::Defaultness,
) {
let (before_predicates, after_predicates) =
generics.where_clause.predicates.split_at(where_clauses.split);
generics.where_clause.predicates.split_at(where_clauses.before_count);
self.head("");
self.print_visibility(vis);
self.print_defaultness(defaultness);
7 changes: 6 additions & 1 deletion compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
@@ -1024,7 +1024,12 @@ impl<'a> Parser<'a> {
has_where_token: after_where_clause.has_where_token,
span: after_where_clause.span,
},
split: before_where_clause.predicates.len(),
before_count: before_where_clause.predicates.len(),
before_with_attr_count: before_where_clause
.predicates
.iter()
.filter(|p| !p.attrs.is_empty())
Copy link
Member

Choose a reason for hiding this comment

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

Filtering things which have attributes applied to them isn't really correct because it's not necessarily the case that all cfgd where clauses will be removed during macro expansion, e.g.

#![feature(where_clause_attrs, cfg_boolean_literals, lazy_type_alias)]
#![expect(incomplete_features)]

struct Foo;
trait Trait {}

impl Trait for Foo {}

type MixedWhereBounds
where
    #[cfg(true)]
    Foo: Trait,
= Foo
where
    (): Sized;

Here we have a where clause before the = that has an attribute applied to it, yet the where clause will still be present after macro expansion.

If you run the compiler on this example with your changes, and then remove the cfg, you'll get different diagnostics output even though post-expansion it'll be the same set of where clauses. That's caused by this before_with_attr_count being 1 and then when we split the where clauses into before/after you wind up splitting at 0 so both clauses end up in the "after" bucket.

Can you add both version as a test (with/without cfg(true))?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not really sure what the correct way to implement this is 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for your detailed explanation, I will add a few tests first.

.count(),
};
let mut predicates = before_where_clause.predicates;
predicates.extend(after_where_clause.predicates);
2 changes: 1 addition & 1 deletion src/tools/rustfmt/src/items.rs
Original file line number Diff line number Diff line change
@@ -1768,7 +1768,7 @@ fn rewrite_ty<R: Rewrite>(
let (before_where_predicates, after_where_predicates) = generics
.where_clause
.predicates
.split_at(where_clauses.split);
.split_at(where_clauses.before_count);
result.push_str(&format!("{}type ", format_visibility(context, vis)));
let ident_str = rewrite_ident(context, ident);

30 changes: 30 additions & 0 deletions tests/ui/where-clauses/cfg-attr-issue-138010-1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#![feature(lazy_type_alias)]
//~^ WARNING: the feature `lazy_type_alias` is incomplete and may not be safe to use and/or cause compiler crashes [incomplete_features]

trait TraitA {}
trait TraitB {}

fn this(_x: &()) {}

fn needs_copy<T>() {
type F<T>
where
//~^ ERROR: where clauses are not allowed before the type for type aliases
#[cfg(a)]
//~^ ERROR: attributes in `where` clause are unstable
//~| WARNING: unexpected `cfg` condition name: `a`
T: TraitA,
#[cfg(b)]
//~^ ERROR: attributes in `where` clause are unstable
//~| WARNING: unexpected `cfg` condition name: `b`
T: TraitB,
():,
():,
= T;
let x: () = ();
this(&x);
}

fn main() {
needs_copy::<()>();
}
70 changes: 70 additions & 0 deletions tests/ui/where-clauses/cfg-attr-issue-138010-1.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
error: where clauses are not allowed before the type for type aliases
--> $DIR/cfg-attr-issue-138010-1.rs:11:5
|
LL | / where
LL | |
LL | | #[cfg(a)]
... |
LL | | ():,
LL | | ():,
| |____________^
|
= note: see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information
help: move it to the end of the type declaration
|
LL ~
LL ~ = T where ():, ():;
|

error[E0658]: attributes in `where` clause are unstable
--> $DIR/cfg-attr-issue-138010-1.rs:13:9
|
LL | #[cfg(a)]
| ^^^^^^^^^
|
= note: see issue #115590 <https://github.com/rust-lang/rust/issues/115590> for more information
= help: add `#![feature(where_clause_attrs)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

error[E0658]: attributes in `where` clause are unstable
--> $DIR/cfg-attr-issue-138010-1.rs:17:9
|
LL | #[cfg(b)]
| ^^^^^^^^^
|
= note: see issue #115590 <https://github.com/rust-lang/rust/issues/115590> for more information
= help: add `#![feature(where_clause_attrs)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

warning: the feature `lazy_type_alias` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/cfg-attr-issue-138010-1.rs:1:12
|
LL | #![feature(lazy_type_alias)]
| ^^^^^^^^^^^^^^^
|
= note: see issue #112792 <https://github.com/rust-lang/rust/issues/112792> for more information
= note: `#[warn(incomplete_features)]` on by default

warning: unexpected `cfg` condition name: `a`
--> $DIR/cfg-attr-issue-138010-1.rs:13:15
|
LL | #[cfg(a)]
| ^ help: found config with similar value: `target_feature = "a"`
|
= help: expected names are: `FALSE` and `test` and 31 more
= help: to expect this configuration use `--check-cfg=cfg(a)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration
= note: `#[warn(unexpected_cfgs)]` on by default

warning: unexpected `cfg` condition name: `b`
--> $DIR/cfg-attr-issue-138010-1.rs:17:15
|
LL | #[cfg(b)]
| ^
|
= help: to expect this configuration use `--check-cfg=cfg(b)`
= note: see <https://doc.rust-lang.org/nightly/rustc/check-cfg.html> for more information about checking conditional configuration

error: aborting due to 3 previous errors; 3 warnings emitted

For more information about this error, try `rustc --explain E0658`.
33 changes: 33 additions & 0 deletions tests/ui/where-clauses/cfg-attr-issue-138010-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#![feature(where_clause_attrs, cfg_boolean_literals, lazy_type_alias)]
#![expect(incomplete_features)]

struct Foo;
trait Trait {}

impl Trait for Foo {}

type MixedWhereBounds1
where //~ ERROR: where clauses are not allowed before the type for type aliases
#[cfg(true)]
Foo: Trait,
= Foo
where
(): Sized;

type MixedWhereBounds2
where //~ ERROR: where clauses are not allowed before the type for type aliases
#[cfg(false)]
Foo: Trait,
= Foo
where
(): Sized;

type MixedWhereBounds3
where
//~^ ERROR: where clauses are not allowed before the type for type aliases
Foo: Trait,
= Foo
where
(): Sized;

fn main() {}
39 changes: 39 additions & 0 deletions tests/ui/where-clauses/cfg-attr-issue-138010-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
error: where clauses are not allowed before the type for type aliases
--> $DIR/cfg-attr-issue-138010-2.rs:10:1
|
LL | / where
LL | | #[cfg(true)]
LL | | Foo: Trait,
| |_______________^ help: remove this `where`
|
= note: see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information

error: where clauses are not allowed before the type for type aliases
--> $DIR/cfg-attr-issue-138010-2.rs:18:1
|
LL | / where
LL | | #[cfg(false)]
LL | | Foo: Trait,
| |_______________^ help: remove this `where`
|
= note: see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information

error: where clauses are not allowed before the type for type aliases
--> $DIR/cfg-attr-issue-138010-2.rs:26:1
|
LL | / where
LL | |
LL | | Foo: Trait,
| |_______________^
|
= note: see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information
help: move it to the end of the type declaration
|
LL +
LL | = Foo
LL | where
LL ~ (): Sized, Foo: Trait;
|

error: aborting due to 3 previous errors

Loading