-
Notifications
You must be signed in to change notification settings - Fork 893
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
sync subtree #5944
sync subtree #5944
Conversation
Syntactically accept `become` expressions (explicit tail calls experiment) This adds `ast::ExprKind::Become`, implements parsing and properly gates the feature. cc `@scottmcm`
… 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
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 <[email protected]>
<<<<<<< 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤣 I know you mentioned that there were a lot of conflicts. No worries on this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cleaning up orphan processes
It seems that non- |
if aaaaaaaaaaaaaaaaaaaaa | ||
&& aaaaaaaaaaaaaaa | ||
&& aaaaaaaaa | ||
&& let Some(x) = xxxxxxxxxxxx | ||
&& aaaaaaa | ||
&& let None = aaaaaaaaaa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is the formatting that I would have expected
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 |
I actually don't think there will be a change since |
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) |
hmmm I guess 🤔 I spoke too soon: https://github.com/rust-lang/rustfmt/actions/runs/6605606043/job/17940945214 |
Going to look into this |
@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. Here's the explanation for what went wrong. When we're doing the subtree sync we're in an interesting scenario where rustfmt at I think the reason we see diffs in the output is because the binary we built from Lines 149 to 152 in 0bb2acf
I couldn't re-run the CI job directly since you merged and deleted the
|
~~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. |
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. |
2023-10-22T18:34:15.3750500Z Cleaning up orphan processes |
2023-10-22T18:34:15.3750500Z Cleaning up orphan processes |
@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)