Skip to content

Detect empty leading where clauses on type aliases#121326

Merged
bors merged 2 commits intorust-lang:masterfrom
fmease:detect-empty-leading-where-clauses-on-ty-aliases
Mar 1, 2024
Merged

Detect empty leading where clauses on type aliases#121326
bors merged 2 commits intorust-lang:masterfrom
fmease:detect-empty-leading-where-clauses-on-ty-aliases

Conversation

@fmease
Copy link
Member

@fmease fmease commented Feb 20, 2024 •
edited
Loading

  1. commit: refactor the AST of type alias where clauses
    • I could no longer bear the look of .0.1 and .1.0
    • Arguably moving split out of TyAlias into a substruct might not make that much sense from a semantic standpoint since it reprs an index into TyAlias.predicates but it's alright and it cleans up the usage sites of TyAlias
  2. commit: fix an oversight: An empty leading where clause is still a leading where clause
    • semantically reject empty leading where clauses on lazy type aliases
      • e.g., on #![feature(lazy_type_alias)] type X where = ();
    • make empty leading where clauses on assoc types trigger lint deprecated_where_clause_location
      • e.g., impl Trait for () { type X where = (); }

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2024

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 20, 2024
@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2024

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

fmease
fmease commented Feb 20, 2024
View reviewed changes
compiler/rustc_ast_passes/src/errors.rs
#[suggestion_part(code = "{snippet}")]
pub right: Span,

pub enum WhereClauseBeforeTypeAliasSugg {
Copy link
Member Author

@fmease fmease Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The split into Remove & Move was not only done to provide a more accurate diagnostic message for the edge case “two where clauses, a leading and a trailing one, where the leading one is empty” but also to prevent the debug assertion empty span and no suggestions from firing.

fmease
fmease commented Feb 20, 2024
View reviewed changes
compiler/rustc_lint/src/context/diagnostics.rs
db.note(
"see issue #89122 <https://github.com/rust-lang/rust/issues/89122> for more information",
);
BuiltinLintDiagnostics::DeprecatedWhereclauseLocation(sugg) => {
Copy link
Member Author

@fmease fmease Feb 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's sad that I couldn't consolidate the messages of the lint and the error but the bridging between rustc_ast_passes & rustc_lint is too tricky. If rustc_lint_defs (the go-between) had a messages.ftl I could make both crates import the fluent_generated messages but that's currently not the case.

@fmease fmease changed the title Detect empty leading where clauses on type aliases Reject empty leading where clauses on type aliases Feb 20, 2024
@fmease fmease changed the title Reject empty leading where clauses on type aliases Detect empty leading where clauses on type aliases Feb 20, 2024
@fmease fmease force-pushed the detect-empty-leading-where-clauses-on-ty-aliases branch from adf7ddc to dbd70e5 Compare February 20, 2024 06:17
compiler-errors
compiler-errors approved these changes Feb 28, 2024
View reviewed changes
Copy link
Contributor

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

thank you for a thoughtful and well commented pr as always

@compiler-errors
Copy link
Contributor

compiler-errors commented Feb 28, 2024

sorry for taking so long to review xd

@bors r+

@bors
Copy link
Collaborator

bors commented Feb 28, 2024

📌 Commit dbd70e5 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 28, 2024
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Feb 28, 2024
@GuillaumeGomez
Rollup merge of rust-lang#121326 - fmease:detect-empty-leading-where-…
a426716 
…clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
@GuillaumeGomez GuillaumeGomez mentioned this pull request Feb 28, 2024
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 28, 2024
Merged
@bors
Copy link
Collaborator

bors commented Feb 28, 2024

☔ The latest upstream changes (presumably #121489) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Feb 28, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
@bors
Auto merge of rust-lang#121770 - matthiaskrgr:rollup-wdher8r, r=matth…
d3d145e 
…iaskrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#110543 (Make `ReentrantLock` public)
 - rust-lang#121689 ([rustdoc] Prevent inclusion of whitespace character after macro_rules ident)
 - rust-lang#121724 (Use `LitKind::Err` for malformed floats)
 - rust-lang#121735 (pattern analysis: Don't panic when encountering unexpected constructor)
 - rust-lang#121743 (Opportunistically resolve regions when processing region outlives obligations)

Failed merges:

 - rust-lang#121326 (Detect empty leading where clauses on type aliases)
 - rust-lang#121416 (Improve error messages for generics with default parameters)
 - rust-lang#121669 (Count stashed errors again)
 - rust-lang#121723 (Two diagnostic things)

r? `@ghost`
`@rustbot` modify labels: rollup
@fmease fmease force-pushed the detect-empty-leading-where-clauses-on-ty-aliases branch from dbd70e5 to cce8128 Compare February 29, 2024 16:20
@fmease
Copy link
Member Author

fmease commented Feb 29, 2024

@bors r=compiler-errors

@bors
Copy link
Collaborator

bors commented Feb 29, 2024

📌 Commit cce8128 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 29, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 29, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 29, 2024
@bors
Auto merge of rust-lang#121810 - matthiaskrgr:rollup-mawij2g, r=matth…
77be7a3 
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#121326 (Detect empty leading where clauses on type aliases)
 - rust-lang#121464 (rustc: Fix wasm64 metadata object files)
 - rust-lang#121681 (Safe Transmute: Revise safety analysis)
 - rust-lang#121753 (Add proper cfg to keep only one AlignmentEnum definition for different target_pointer_widths)
 - rust-lang#121782 (allow statics pointing to mutable statics)
 - rust-lang#121798 (Fix links in rustc doc)
 - rust-lang#121806 (add const test for ptr::metadata)
 - rust-lang#121809 (Remove doc aliases to PATH)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit dd4ecd1 into rust-lang:master Mar 1, 2024
@rustbot rustbot added this to the 1.78.0 milestone Mar 1, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 1, 2024
@rust-timer
Unrolled build for rust-lang#121326
ec0845b 
Rollup merge of rust-lang#121326 - fmease:detect-empty-leading-where-clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
@fmease fmease deleted the detect-empty-leading-where-clauses-on-ty-aliases branch March 1, 2024 00:52
calebcartwright pushed a commit to calebcartwright/rust that referenced this pull request Jun 22, 2024
@matthiaskrgr
Rollup merge of rust-lang#121326 - fmease:detect-empty-leading-where-…
050610e 
…clauses-on-ty-aliases, r=compiler-errors

Detect empty leading where clauses on type aliases

1. commit: refactor the AST of type alias where clauses
   * I could no longer bear the look of `.0.1` and `.1.0`
   * Arguably moving `split` out of `TyAlias` into a substruct might not make that much sense from a semantic standpoint since it reprs an index into `TyAlias.predicates` but it's alright and it cleans up the usage sites of `TyAlias`
2. commit: fix an oversight: An empty leading where clause is still a leading where clause
   * semantically reject empty leading where clauses on lazy type aliases
     * e.g., on `#![feature(lazy_type_alias)] type X where = ();`
   * make empty leading where clauses on assoc types trigger lint `deprecated_where_clause_location`
     * e.g., `impl Trait for () { type X where = (); }`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@compiler-errors compiler-errors compiler-errors approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@compiler-errors compiler-errors

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.78.0

Development

Successfully merging this pull request may close these issues.

4 participants

@fmease @rustbot @compiler-errors @bors