Skip to content

Conversation

pull[bot]
Copy link

@pull pull bot commented Jun 20, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@pull pull bot added the ⤵️ pull label Jun 20, 2023
ytmimi and others added 29 commits June 21, 2024 16:57
Moved the link reference definitions closer to where they were being
used and moved the `type ascription` formatting fix to the correct
section.
Merge `PatParam`/`PatWithOr`, and `Expr`/`Expr2021`, for a few reasons.

- It's conceptually nice, because the two pattern kinds and the two
  expression kinds are very similar.

- With expressions in particular, there are several places where both
  expression kinds get the same treatment.

- It removes one unreachable match arm.

- Most importantly, for #124141 I will need to introduce a new type
  `MetaVarKind` that is very similar to `NonterminalKind`, but records a
  couple of extra fields for expression metavars. It's nicer to have a
  single `MetaVarKind::Expr` expression variant to hold those extra
  fields instead of duplicating them across two variants
  `MetaVarKind::{Expr,Expr2021}`. And then it makes sense for patterns
  to be treated the same way, and for `NonterminalKind` to also be
  treated the same way.

I also clarified the comments, because I have long found them a little
hard to understand.
…r-errors

Rework pattern and expression nonterminal kinds.

Some tweaks to `NonterminalKind` that will assist with #124141. Details in the individual commits.

r? compiler-errors
cc ```@eholk```
Implement `use<>` formatting in rustfmt

This PR implements formatting for precise-capturing `use<>` syntax as proposed in rust-lang/rust#126753.

The syntax is implemented as-if the `use<>` bound were a trait bound but with the `use` keyword as its path segment identifier.

I opted to develop this in the rust-lang/rust tree since I'm not certain when the next rustfmt subtree sync is going to be, and I'd rather not block landing nightly support for `use<>` on something I have no control over. If ``@rust-lang/rustfmt`` would rather I move this PR over to that repository, then I would at least like to know when the next rustfmt->rust subtree sync is going to be, since stabilizing `precise_capturing` without formatting will be disruptive.

This implementation is otherwise rather straightforward.

Tracking:

- rust-lang/rust#123432
Remove stray println from rustfmt's `rewrite_static`

r? `@calebcartwright` `@ytmimi` -- though anyone should probably r+ this so it gets into nightly sooner than later, since it's obviously wrong.

This can just be fixed in-tree, since I don't think we want to wait until the next sync to fix this.

Fix #6210
Fix rust-lang/rust#126887
impl rewrite_result for ast::Local, ast::FieldDef, ast::Param, ast::FnRetTy
fmease and others added 30 commits March 24, 2025 20:04
Since `{ ident: ident }` is a parse error, these fields are dead.
Visitors track whether an assoc item is in a trait impl or an inherent impl

`AssocCtxt::Impl` now contains an `of_trait` field. This allows ast lowering and nameres to not have to track whether we're in a trait impl or an inherent impl.
…mpiler-errors

Mostly parser: Eliminate code that's been dead / semi-dead since the removal of type ascription syntax

**Disclaimer**: This PR is intended to mostly clean up code as opposed to bringing about behavioral changes. Therefore it doesn't aim to address any of the 'FIXME: remove after a month [dated: 2023-05-02]: "type ascription syntax has been removed, see issue [#]101728"'.

---

By commit:

1. Removes truly dead code:
   * Since 1.71 (#109128) `let _ = { f: x };` is a syntax error as opposed to a semantic error which allows the parse-time diagnostic (suggestion) "*struct literal body without path // you might have forgotten […]*" to kick in.
   * The analysis-time diagnostic (suggestion) from <=1.70 "*cannot find value \`f\` in this scope // you might have forgotten […]*" is therefore no longer reachable.
2. Updates `is_certainly_not_a_block` to be in line with the current grammar:
   * The seq. `{ ident:` is definitely not the start of a block. Before the removal of ty ascr, `{ ident: ty_start` would begin a block expr.
   * This shouldn't make more code compile IINM, it should *ultimately* only affect diagnostics.
   * For example, `if T { f: () } {}` will now be interpreted as an `if` with struct lit `T { f: () }` as its *condition* (which is banned in the parser anyway) as opposed to just `T` (with the *consequent* being `f : ()` which is also invalid (since 1.71)). The diagnostics are almost the same because we have two separate parse recovery procedures + diagnostics: `StructLiteralNeedingParens` (*invalid struct lit*) before and `StructLiteralNotAllowedHere` (*struct lits aren't allowed here*) now, as you can see from the diff.
   * (As an aside, even before this PR, fn `maybe_suggest_struct_literal` should've just used the much older & clearer `StructLiteralNotAllowedHere`)
   * NB: This does sadly regress the compiler output for `tests/ui/parser/type-ascription-in-pattern.rs` but that can be fixed in follow-up PRs. It's not super important IMO and a natural consequence.
3. Removes code that's become dead due to the prior commit.
   * Basically reverts #106620 + #112475 (without regressing rustc's output!).
   * Now the older & more robust parse recovery procedure (cc `StructLiteralNotAllowedHere`) takes care of the cases the removed code used to handle.
   * This automatically fixes the suggestions for \[[playground](https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=7e2030163b11ee96d17adc3325b01780)\]:
     * `if Ty::<i32> { f: K }.m() {}`: `if Ty::<i32> { SomeStruct { f: K } }.m() {}` (broken) → ` if (Ty::<i32> { f: K }).m() {}`
     * `if <T as Trait>::Out { f: K::<> }.m() {}`: `if <T as Trait>(::Out { f: K::<> }).m() {}` (broken) → `if (<T as Trait>::Out { f: K::<> }).m() {}`
4. Merge and simplify UI tests pertaining to this issue, so it's easier to add more regression tests like for the two cases mentioned above.
5. Merge UI tests and add the two regression tests.

Best reviewed commit by commit (on request I'll partially squash after approval).
Instead of putting the item inside it, just pass the ident and
visibility (the only things needed) alongside it where necessary.

This helps with the next commit, which will move the ident's location.
Specifically, it gets rid of the `match visitor_kind` in
`rewrite_type_alias`.
`FmtVisitor::visit_mac` has an `Option<Ident>` arg which is always
either `None` or `Some(kw::Empty)`, because `ItemKind::MacCall` always
has an empty ident. This value is passed through various functions until
it reaches `rewrite_macro_name`, which treats `None` and
`Some(kw::Empty)` the same.

In other words, the argument is useless. This commit removes it. There
is no change in behaviour. The commit also changes a few `symbol::Ident`
occurrences to `Ident` in `macros.rs`; `Symbol` is imported in that file
so `Ident` might as well be, too.

(This is a good example of why it's a bad idea for `Itemt` to have an
`ident` field when various item kinds don't have an identifier. It's
easy to get confused when "empty identifier" is used to mean "no
identifier". This will be fixed in a subsequent commit.)
`ast::Item` has an `ident` field.

- It's always non-empty for these item kinds: `ExternCrate`, `Static`,
  `Const`, `Fn`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`,
  `Trait`, `TraitAlias`, `MacroDef`, `Delegation`.

- It's always empty for these item kinds: `Use`, `ForeignMod`,
  `GlobalAsm`, `Impl`, `MacCall`, `DelegationMac`.

There is a similar story for `AssocItemKind` and `ForeignItemKind`.

Some sites that handle items check for an empty ident, some don't. This
is a very C-like way of doing things, but this is Rust, we have sum
types, we can do this properly and never forget to check for the
exceptional case and never YOLO possibly empty identifiers (or possibly
dummy spans) around and hope that things will work out.

The commit is large but it's mostly obvious plumbing work. Some notable
things.

- `ast::Item` got 8 bytes bigger. This could be avoided by boxing the
  fields within some of the `ast::ItemKind` variants (specifically:
  `Struct`, `Union`, `Enum`). I might do that in a follow-up; this
  commit is big enough already.

- For the visitors: `FnKind` no longer needs an `ident` field because
  the `Fn` within how has one.

- In the parser, the `ItemInfo` typedef is no longer needed. It was used
  in various places to return an `Ident` alongside an `ItemKind`, but
  now the `Ident` (if present) is within the `ItemKind`.

- In a few places I renamed identifier variables called `name` (or
  `foo_name`) as `ident` (or `foo_ident`), to better match the type, and
  because `name` is normally used for `Symbol`s. It's confusing to see
  something like `foo_name.name`.
remove `feature(inline_const_pat)`

Summarizing https://rust-lang.zulipchat.com/#narrow/channel/144729-t-types/topic/remove.20feature.28inline_const_pat.29.20and.20shared.20borrowck.

With rust-lang/types-team#129 we will start to borrowck items together with their typeck parent. This is necessary to correctly support opaque types, blocking the new solver and TAIT/ATPIT stabilization with the old one. This means that we cannot really support `inline_const_pat` as they are implemented right now:

- we want to typeck inline consts together with their parent body to allow inference to flow both ways and to allow the const to refer to local regions of its parent.This means we also need to borrowck the inline const together with its parent as that's necessary to properly support opaque types
- we want the inline const pattern to participate in exhaustiveness checking
- to participate in exhaustiveness checking we need to evaluate it, which requires borrowck, which now relies on borrowck of the typeck root, which ends up checking exhaustiveness again. **This is a query cycle**.

There are 4 possible ways to handle this:
- stop typechecking inline const patterns together with their parent
  - causes inline const patterns to be different than inline const exprs
  - prevents bidirectional inference, we need to either fail to compile `if let const { 1 } = 1u32` or `if let const { 1u32 } = 1`
  - region inference for inline consts will be harder, it feels non-trivial to support inline consts referencing local regions from the parent fn
- inline consts no longer participate in exhaustiveness checking. Treat them like `pat if pat == const { .. }`  instead. We then only evaluate them after borrowck
  - difference between `const { 1 }`  and `const FOO: usize = 1; match x { FOO => () }`. This is confusing
  - do they carry their weight if they are now just equivalent to using an if-guard
- delay exhaustiveness checking until after borrowck
  - should be possible in theory, but is a quite involved change and may have some unexpected challenges
- remove this feature for now

I believe we should either delay exhaustiveness checking or remove the feature entirely. As moving exhaustiveness checking to after borrow checking is quite complex I think the right course of action is to fully remove the feature for now and to add it again once/if we've got that implementation figured out.

`const { .. }`-expressions remain stable. These seem to have been the main motivation for rust-lang/rfcs#2920.

r? types

cc `@rust-lang/types` `@rust-lang/lang` #76001
As of the stabalization of edition 2024 (including style edition 2024)
the `overflow_delimited_expr` default was reverted back to `false`.

rustfmt hasn't been synced with r-l/rust in a while and not having this
config is causing massive diffs in rustfmt's `Diff Check` job.

To make the next subtree push easier to verify I'm manually adding this
change to the rustfmt tree before the `subtree push`.
Bumping the toolchain version as part of a git subtree push

current toolchain (nightly-2025-01-02):
  - 1.85.0-nightly (45d11e51b 2025-01-01)

latest toolchain (nightly-2025-04-02):
  - 1.88.0-nightly (e2014e876 2025-04-01)
Tracking issue r-l/rust#134797

Closes 6512
Allows to disable the indentation of match arms.

Related issue: #2937

Co-authored-by: Yacin Tmimi <[email protected]>
It did not include `b` for the `use_field_init_shorthand = true` case,
which is a mistake as it is the one thing that will be changed by this
option.
Locate `rustfmt` in the same directory as `cargo-fmt` when the RUSTFMT env isn't set. 

Signed-off-by: onur-ozkan <[email protected]>
…lf::self;` (#6573)

This prevents an idempotence issue where each run of rustfmt removed another `self` from the import path.
For example, `use self::self::self;` -> `use self::self;` -> `use self;` -> `{import removed}`.

Now `use self::self;`, `use self::self::self`, etc. are all left unchanged, and users will get a compiler error if they try to compile code with these invalid imports.

This change does not impact normalizing `use foo::self;` -> `use foo;`.
Most notably this release stabalizes `style_edition=2024` and the
`--style-edition` CLI option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.