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.

Sign up for GitHub

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

Jump to bottom

Fix #96847 #100307

Merged
merged 2 commits into from
Aug 12, 2022
Merged

Fix #96847 #100307

merged 2 commits into from
Aug 12, 2022

Conversation

nnethercote
Copy link
Contributor

@nnethercote
Move JSON tests into a directory.
c32969d 
To get around the "following path contains more than 968 entries, you
should move the test to some relevant subdirectory" tidy error.
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 9, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2022
@nnethercote
Copy link
Contributor Author

The fix is simple, but I admit I don't understand why it's needed. This attribute gets lowered twice, with the second time now being a no-op. If I remove the loop the double lowering goes away.

@cjgillot
Copy link
Contributor

cjgillot commented Aug 9, 2022

I think that the double lowering happens in rustc_ast_lowering/expr.rs, line 1538. We should pass e.attrs directly to expr_drop_temps_mut.

@nnethercote
Copy link
Contributor Author

nnethercote commented Aug 9, 2022 •
edited
Loading

Thanks, @cjgillot, you were right! Your punishment for this good deed:

r? @cjgillot

😄

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 10, 2022

📌 Commit 6503bbd2eb89d3b35af13a2274e62dec4743c9fb has been approved by cjgillot

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 Aug 10, 2022
@nnethercote
Copy link
Contributor Author

I changed the commit message on the second commit to reflect the change.

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Aug 11, 2022

📌 Commit ce78042 has been approved by cjgillot

It is now in the queue for this repository.

@nnethercote
Copy link
Contributor Author

@bors rollup=always

@matthiaskrgr matthiaskrgr mentioned this pull request Aug 11, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 11, 2022
@bors
Auto merge of rust-lang#100426 - matthiaskrgr:rollup-0ks4dou, r=matth…
2ed0f29 
…iaskrgr

Rollup of 13 pull requests

Successful merges:

 - rust-lang#93896 (rustdoc: make item-infos dimmer on dark theme)
 - rust-lang#99337 (rustdoc: simplify highlight.rs)
 - rust-lang#99421 (add crt-static for android)
 - rust-lang#99500 (Fix flags when using clang as linker for Fuchsia)
 - rust-lang#99511 (make raw_eq precondition more restrictive)
 - rust-lang#99992 (Add `x.sh` and `x.ps1` shell scripts)
 - rust-lang#100112 (Fix test: chunks_mut_are_send_and_sync)
 - rust-lang#100203 (provide correct size hint for unsupported platform `CommandArgs`)
 - rust-lang#100307 (Fix rust-lang#96847)
 - rust-lang#100350 (Stringify non-shorthand visibility correctly)
 - rust-lang#100374 (Improve crate selection on rustdoc search results page)
 - rust-lang#100392 (Simplify visitors)
 - rust-lang#100418 (Add stability attributes to BacktraceStatus variants)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e221aaf into rust-lang:master Aug 12, 2022
@rustbot rustbot added this to the 1.65.0 milestone Aug 12, 2022
@nnethercote nnethercote added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 12, 2022
@nnethercote
Copy link
Contributor Author

Nominating for beta backport. It's an obscure bug unlikely to be hit by human-written code (the bug was found by a fuzzer) but the fix is simple enough that it might be worth getting it out 6 weeks earlier.

@nnethercote nnethercote deleted the fix-96847 branch August 12, 2022 03:14
@pnkfelix
Copy link
Member

pnkfelix commented Aug 18, 2022 •
edited
Loading

Discussed at T-compiler meeting.

Declining to backport; the scope of this problem, from brief investigation, seems limited to inner attributes (#![...] not #[...]) seems limited to attributes on ... for loops? We think this can ride the trains.

@apiraino
Copy link
Contributor

Beta backport declined as per compiler team on Zulip

@rustbot label -beta-nominated

@rustbot rustbot removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Aug 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cjgillot cjgillot

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.65.0
Development

Successfully merging this pull request may close these issues.
8 participants
@nnethercote @cjgillot @bors @pnkfelix @apiraino @petrochenkov @rust-highfive @rustbot