sync subtree #5944

calebcartwright · 2023-10-22T18:00:57Z

@ytmimi I'll be intermittently away from my machine today so if you're around it'd be helpful if you could keep an eye on status and ping me/merge as necessary

(though remember that for sync PRs we never squash nor rebase the merge commits)

…sync

Syntactically accept `become` expressions (explicit tail calls experiment) This adds `ast::ExprKind::Become`, implements parsing and properly gates the feature. cc `@scottmcm`

…tfmt

… have a `ProjectionCandidate`

Implement rust-lang/compiler-team#578. When an ICE is encountered on nightly releases, the new rustc panic handler will also write the contents of the backtrace to disk. If any `delay_span_bug`s are encountered, their backtrace is also added to the file. The platform and rustc version will also be collected.

…ly built in-tree version

Currently, Clippy, Miri, Rustfmt, and rustc all use an environment variable to indicate that output should be blessed, but they use different variable names. In order to improve consistency, this patch applies the following changes: - Emit `RUSTC_BLESS` within `prepare_cargo_test` so it is always available - Change usage of `MIRI_BLESS` in the Miri subtree to use `RUSTC_BLESS` - Change usage of `BLESS` in the Clippy subtree to `RUSTC_BLESS` - Change usage of `BLESS` in the Rustfmt subtree to `RUSTC_BLESS` - Adjust the blessable test in `rustc_errors` to use this same convention - Update documentation where applicable Any tools that uses `RUSTC_BLESS` should check that it is set to any value other than `"0"`.

Token tree cloning is only needed in one place.

…henkov Less `TokenTree` cloning `TokenTreeCursor` has this comment on it: ``` // FIXME: Many uses of this can be replaced with by-reference iterator to avoid clones. ``` This PR completes that FIXME. It doesn't have much perf effect, but at least we now know that. r? `@petrochenkov`

discard default auto trait impls if explicit ones exist (rebase of #85048) Rebase of #85048

…new`

It's the same as `Delimiter`, minus the `Invisible` variant. I'm generally in favour of using types to make impossible states unrepresentable, but this one feels very low-value, and the conversions between the two types are annoying and confusing. Look at the change in `src/tools/rustfmt/src/expr.rs` for an example: the old code converted from `MacDelimiter` to `Delimiter` and back again, for no good reason. This suggests the author was confused about the types.

Suggests turbofish in patterns Fixes #114112 r? ```@estebank```

Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location. This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR.

Lots of tiny incremental simplifications of `EmitterWriter` internals ignore the first commit, it's rust-lang/rust#114088 squashed and rebased, but it's needed to use to use `derive_setters`, as they need a newer `syn` version. Then this PR starts out with removing many arguments that are almost always defaulted to `None` or `false` and replace them with builder methods that can set these fields in the few cases that want to set them. After that it's one commit after the other that removes or merges things until everything becomes some very simple trait objects

Improve spans for indexing expressions fixes #114388 Indexing is similar to method calls in having an arbitrary left-hand-side and then something on the right, which is the main part of the expression. Method calls already have a span for that right part, but indexing does not. This means that long method chains that use indexing have really bad spans, especially when the indexing panics and that span in coverted into a panic location. This does the same thing as method calls for the AST and HIR, storing an extra span which is then put into the `fn_span` field in THIR. r? compiler-errors

Rollup of 9 pull requests Successful merges: - #113945 (Fix wrong span for trait selection failure error reporting) - #114351 ([rustc_span][perf] Remove unnecessary string joins and allocs.) - #114418 (bump parking_lot to 0.12) - #114434 (Improve spans for indexing expressions) - #114450 (Fix ICE failed to get layout for ReferencesError) - #114461 (Fix unwrap on None) - #114462 (interpret: add mplace_to_ref helper method) - #114472 (Reword `confusable_idents` lint) - #114477 (Account for `Rc` and `Arc` when suggesting to clone) r? `@ghost` `@rustbot` modify labels: rollup

Anonymous structs or unions are only allowed in struct field definitions. Co-authored-by: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com>

…-10-22

calebcartwright · 2023-10-22T18:04:00Z

CHANGELOG.md

+<<<<<<< HEAD
+[#5690]: (https://github.com/rust-lang/rustfmt/pulls/5690)
+=======
 [#5690]: https://github.com/rust-lang/rustfmt/pull/5690
+>>>>>>> upstream/master
 [#5684]: https://github.com/rust-lang/rustfmt/issues/5684


Ugh 🤮

I clearly got tired of merge conflicts. Mercifully this is just noise in the changelog, so i'll clean this up later when I add the additional entries

🤣 I know you mentioned that there were a lot of conflicts. No worries on this!

Cleaning up orphan processes

calebcartwright · 2023-10-22T18:30:06Z

It seems that non-&& operators fail during parsing now as opposed to ast validation stage, so we'll need to update the let chains tests accordingly

calebcartwright · 2023-10-22T18:31:37Z

tests/target/let_chains.rs

+    if aaaaaaaaaaaaaaaaaaaaa
+        && aaaaaaaaaaaaaaa
+        && aaaaaaaaa
+        && let Some(x) = xxxxxxxxxxxx
+        && aaaaaaa
+        && let None = aaaaaaaaaa


I do have to step away now for a while, but wanted to flag this for your attention. Given that all the operators are now the same this does appear to be the correct formatting to my mind, do you agree?

Yeah, this is the formatting that I would have expected

ytmimi · 2023-10-22T18:51:01Z

Just for good measure I'm running the Diff Check Job

calebcartwright · 2023-10-22T18:54:23Z

Just for good measure I'm running the Diff Check Job

Good, I'd been meaning to suggest we do so just to get a feel for the let chain results. Obviously we should be expecting some level of diff in just about every repo that has let chains in the code

ytmimi · 2023-10-22T19:00:45Z

I actually don't think there will be a change since let-chain support has already been merged, and we compare results between running the latest rustfmt from source and this PRs version of rustfmt. As long as both rustfmt binaries produce the same diff then we consider that a pass. That being said, I can run the diff check version from #5827 and inspect the diffs in the zipfile since we'll still capture those.

calebcartwright · 2023-10-22T19:07:51Z

I'm going to go ahead and merge this then. If you have bandwidth now/within the next couple hours to do the version bump and changelog that'd also be a big help 🙏 (a single commit that adds the new entries to the changelog + another pass to fix some typos, and then does a patch version bump in the cargo.toml + lockfile files would do it)

ytmimi · 2023-10-22T19:08:40Z

hmmm I guess 🤔 I spoke too soon: https://github.com/rust-lang/rustfmt/actions/runs/6605606043/job/17940945214

ytmimi · 2023-10-22T19:08:45Z

Going to look into this

ytmimi · 2023-10-22T21:00:35Z

@calebcartwright Okay, I think I know why the diff-check job failed, and I'm happy to say that it was a false positive. When I ran things locally, everything checked out 🙏🏼

You can inspect the zip if you'd like.

diff-check.zip

Here's the explanation for what went wrong.

When we're doing the subtree sync we're in an interesting scenario where rustfmt at origin/master and rustfmt from the feature branch (in this case subtree-sync-2023-10-22) need different LD_LIBRARY_PATHs to be set since the subtree sync modifies the rust-toolchain file.

I think the reason we see diffs in the output is because the binary we built from subtree-sync-2023-10-22 failed to run at all, and we silence those issues with set +e:

rustfmt/ci/check_diff.sh

Lines 149 to 152 in 0bb2acf

    
           # rustfmt --check returns 1 if a diff was found 
        
           # Also check_diff returns 1 if there was a diff between master rustfmt and the feature branch 
        
           # so we want to ignore the exit status check 
        
           set +e

I couldn't re-run the CI job directly since you merged and deleted the subtree-sync-2023-10-22 branch, so I needed to work around that when running the script locally. Here are the steps you could take to reproduce the successful diff-check run:

created a subtree-sync-2023-10-22 branch in my fork of the repo where the HEAD is in line with origin/master (0bb2acf)
checked out ytmimi:diff-check-export

applied this patch to modify how we set LD_LIBRARY_PATH and hard code 547577f as the master rustfmt:

diff --git a/ci/check_diff.sh b/ci/check_diff.sh
index b6ce62c2..c5ea2792 100755
--- a/ci/check_diff.sh
+++ b/ci/check_diff.sh
@@ -2,9 +2,6 @@

 set -e

-# https://github.com/rust-lang/rustfmt/issues/5675
-export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH
-
function print_usage() {
     echo "usage check_diff REMOTE_REPO FEATURE_BRANCH [COMMIT_HASH] [OPTIONAL_RUSTFMT_CONFIGS]"
}
@@ -127,6 +124,13 @@ function compile_rustfmt() {
     echo -e "\ncompiling with $CARGO_VERSON\n"

     echo "Building rustfmt from src"
+    # rustfmt commit before the latest subtree sync (https://github.com/rust-lang/rustfmt/pull/5944)
+    echo "checking out commit hash 547577fa5d309d90292ca3a58fef1bf0d9325cc0"
+    git fetch origin 547577fa5d309d90292ca3a58fef1bf0d9325cc0
+    git checkout --detach 547577fa5d309d90292ca3a58fef1bf0d9325cc0
+    # Add the LD_LIBRARY_PATH for master rustfmt
+    export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH
+    echo "LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
     cargo build -q --release --bin rustfmt && cp target/release/rustfmt $1/rustfmt

     if [ -z "$OPTIONAL_COMMIT_HASH" ] || [ "$FEATURE_BRANCH" = "$OPTIONAL_COMMIT_HASH" ]; then
@@ -136,6 +140,11 @@ function compile_rustfmt() {
     fi

     echo "Building feature rustfmt from src"
+    # Add the `LD_LIBRARY_PATH` for feature rustfmt. In most cases the `LD_LIBRARY_PATH` should be the same,
+    # for both binaries, however during subtree syncs we bump the `channel` in the rust-toolchain file, and
+    # therefore the feature branch needs to use a different sysroot.
+    export LD_LIBRARY_PATH=$(rustc --print sysroot)/lib:$LD_LIBRARY_PATH
+    echo "LD_LIBRARY_PATH: $LD_LIBRARY_PATH"
     cargo build -q --release --bin rustfmt && cp target/release/rustfmt $1/feature_rustfmt

     RUSFMT_BIN=$1/rustfmt

run the script:

./ci/check_diff.sh https://github.com/ytmimi/rustfmt.git subtree-sync-2023-10-22 subtree-sync-2023-10-22

calebcartwright · 2023-10-22T21:35:33Z

~~Would it help if I restored the branch? ~~

Nevermind, it's been too long and GitHub no longer allows me to restore. I really appreciate how deeply you dug into this.

I am still puzzled though why the sysroot location produces different formatting results? Or is it more that one of the two points of comparison didn't have any rustfmt execution (e.g. original_unformatted_input vs. output_from_actually_formatting_input)

ytmimi · 2023-10-22T22:05:59Z

I don't think restoring the branch would have made much of a difference anyway.

The issue was that only the master rustfmt binary ran successfully. When it ran it produced diffs, but the feature branch binary failed to run because of the runtime dependency on a different sysroot so no diffs were produced, and the diff-check script saw that discrepancy and marked the job as a failure.

I'll open up a PR with a similar patch to avoid these issues for future sub tree syncs moving forward.

yorch2221 · 2023-11-26T18:08:39Z

2023-10-22T18:34:15.3750500Z Cleaning up orphan processes

yorch2221 · 2023-11-26T18:08:56Z

@ytmimi I'll be intermittently away from my machine today so if you're around it'd be helpful if you could keep an eye on status and ping me/merge as necessary

(though remember that for sync PRs we never squash nor rebase the merge commits)

2023-10-22T18:34:15.3750500Z Cleaning up orphan processes

WaffleLapkin and others added 29 commits June 19, 2023 12:54

Implement become expression formatting in rustfmt

dc7dc3e

Merge commit '3f7c366fc0464e01ddcaefbd70647cb3da4202be' into rustfmt-…

ed3e96e

…sync

Rollup merge of #112790 - WaffleLapkin:syntactically, r=Nilstrieb

80bf3ea

Syntactically accept `become` expressions (explicit tail calls experiment) This adds `ast::ExprKind::Become`, implements parsing and properly gates the feature. cc `@scottmcm`

Merge commit 'dca1cf90ad6b8e45afbed2061803befbb2d159e9' into sync-rus…

78331ee

…tfmt

always emit consider AutoImplCandidates for them if they don't also…

d0762f0

… have a `ProjectionCandidate`

Use a builder instead of boolean/option arguments

a3dfd82

Make x test src/tools/rustfmt --bless format rustfmt with the fresh…

c0ee8f5

…ly built in-tree version

Avoid into_trees usage in rustfmt.

e60a9e2

Token tree cloning is only needed in one place.

Auto merge of #113312 - Ddystopia:auto-trait-fun, r=lcnr

4eeecae

discard default auto trait impls if explicit ones exist (rebase of #85048) Rebase of #85048

Use builder pattern instead of lots of arguments for `EmitterWriter::…

f122a33

…new`

Fix rustfmt dep

e42bc9e

Rollup merge of #114300 - MU001999:fix/turbofish-pat, r=estebank

2c8cc96

Suggests turbofish in patterns Fixes #114112 r? ```@estebank```

Parse unnamed fields and anonymous structs or unions

cef3117

Anonymous structs or unions are only allowed in struct field definitions. Co-authored-by: carbotaniuman <41451839+carbotaniuman@users.noreply.github.com>

Use conditional synchronization for Lock

9c25fbf

Use relative positions inside a SourceFile.

96e583f

Update tools and fulldeps tests

b8e3cb0

Fix spans for comments in rustfmt

8ac561a

s/generator/coroutine/

0f73981

Rename lots of files that had generator in their name

4b5ef37

Merge remote-tracking branch 'upstream/master' into subtree-sync-2023…

f35f252

…-10-22

chore: bump toolchain and apply minor updates

746bf48

rustbot added the pr-not-reviewed label Oct 22, 2023

calebcartwright commented Oct 22, 2023

View reviewed changes

deps: bump bytecount version

75d8497

tests: fix let chain tests

c2515df

calebcartwright commented Oct 22, 2023

View reviewed changes

ytmimi approved these changes Oct 22, 2023

View reviewed changes

calebcartwright merged commit 0bb2acf into rust-lang:master Oct 22, 2023
27 checks passed

calebcartwright deleted the subtree-sync-2023-10-22 branch October 22, 2023 19:08

calebcartwright removed the pr-not-reviewed label Oct 22, 2023

yorch2221 approved these changes Nov 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sync subtree #5944

sync subtree #5944

calebcartwright commented Oct 22, 2023

calebcartwright Oct 22, 2023

ytmimi Oct 22, 2023 •

edited

Loading

yorch2221 Nov 26, 2023

calebcartwright commented Oct 22, 2023

calebcartwright Oct 22, 2023

ytmimi Oct 22, 2023

ytmimi commented Oct 22, 2023

calebcartwright commented Oct 22, 2023

ytmimi commented Oct 22, 2023 •

edited

Loading

calebcartwright commented Oct 22, 2023

ytmimi commented Oct 22, 2023

ytmimi commented Oct 22, 2023

ytmimi commented Oct 22, 2023

calebcartwright commented Oct 22, 2023 •

edited

Loading

ytmimi commented Oct 22, 2023

yorch2221 commented Nov 26, 2023

yorch2221 commented Nov 26, 2023

sync subtree #5944

sync subtree #5944

Conversation

calebcartwright commented Oct 22, 2023

calebcartwright Oct 22, 2023

Choose a reason for hiding this comment

ytmimi Oct 22, 2023 • edited Loading

Choose a reason for hiding this comment

yorch2221 Nov 26, 2023

Choose a reason for hiding this comment

calebcartwright commented Oct 22, 2023

calebcartwright Oct 22, 2023

Choose a reason for hiding this comment

ytmimi Oct 22, 2023

Choose a reason for hiding this comment

ytmimi commented Oct 22, 2023

calebcartwright commented Oct 22, 2023

ytmimi commented Oct 22, 2023 • edited Loading

calebcartwright commented Oct 22, 2023

ytmimi commented Oct 22, 2023

ytmimi commented Oct 22, 2023

ytmimi commented Oct 22, 2023

calebcartwright commented Oct 22, 2023 • edited Loading

ytmimi commented Oct 22, 2023

yorch2221 commented Nov 26, 2023

yorch2221 commented Nov 26, 2023

ytmimi Oct 22, 2023 •

edited

Loading

ytmimi commented Oct 22, 2023 •

edited

Loading

calebcartwright commented Oct 22, 2023 •

edited

Loading