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.

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

Fix uninlined_format_args in compiler crates with the diagnostic migration completed #105890

Merged
merged 1 commit into from
Jan 6, 2023

Conversation

Noratrieb
Copy link
Member

@Noratrieb Noratrieb commented Dec 19, 2022

Convert all the crates that have had their diagnostic migration completed (except save_analysis because that will be deleted soon and apfloat because of the licensing problem).

Some of them have been reviewed by myself and they were all correct (though I still recommend going over all of them again for review).

@rustbot
Copy link
Collaborator

rustbot commented Dec 19, 2022

r? @jackh726

(rustbot has picked a reviewer for you, use r? to override)

@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 Dec 19, 2022
@@ -327,11 +327,11 @@ fn print_target_features(sess: &Session, tm: &llvm::TargetMachine) {

println!("Features supported by rustc for this target:");
for (feature, desc) in &rustc_target_features {
println!(" {1:0$} - {2}.", max_feature_len, feature, desc);
println!(" {feature:max_feature_len$} - {desc}.");
Copy link
Member Author

Choose a reason for hiding this comment

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

That's a fun one, you rarely see it. Looks correct though.

@Noratrieb
Copy link
Member Author

Noratrieb commented Dec 19, 2022

(OUTDATED)
I looked through all the changes and they look fine, as expected.

One problem with this is that this will probably cause a bunch of conflicts with diagnostic migrations which is pretty bad. I'm not sure what the best way to proceed is here. Maybe it makes sense to only do the change on crates that have their diagnostics fully migrated?

That does sound like a decent idea to me, I'll do it.

@Noratrieb Noratrieb marked this pull request as ready for review December 19, 2022 08:38
@rustbot
Copy link
Collaborator

rustbot commented Dec 19, 2022

Changes rustc_apfloat. rustc_apfloat is currently in limbo and you almost certainly don't want to change it (see #55993).

cc @eddyb

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@matthiaskrgr
Copy link
Member

There are some problems regarding refactorings with RA and inline args, which lead clippy to consider disabling the lint by default.
Not sure if you want to land this right now

https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60uninlined_format_args.60.20category

@Noratrieb Noratrieb marked this pull request as draft December 19, 2022 09:22
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic labels Dec 19, 2022
@Noratrieb Noratrieb changed the title Fix uninlined_format_args in rustc_(a|b|c)* crates Fix uninlined_format_args in compiler crates with the diagnostic migration completed Dec 19, 2022
@Noratrieb Noratrieb marked this pull request as ready for review December 19, 2022 09:39
@rustbot
Copy link
Collaborator

rustbot commented Dec 19, 2022

rustc_macros::diagnostics was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

These commits modify compiler targets.
(See the Target Tier Policy.)

@Noratrieb
Copy link
Member Author

As a user for rust-analyzer, the problems aren't bad enough to not do this now in my opinion, but I could also understand it if we deem it bad enough and want to delay it.

@bors
Copy link
Contributor

bors commented Dec 20, 2022

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

@RalfJung
Copy link
Member

I don't understand why the bot pinged me, I see no interpreter changes?

@Noratrieb
Copy link
Member Author

I had some at first that I then removed.

@Noratrieb Noratrieb 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 3, 2023
@jackh726
Copy link
Member

jackh726 commented Jan 4, 2023

r=me after rebase

@bors delegate+

@bors
Copy link
Contributor

bors commented Jan 4, 2023

✌️ @Nilstrieb can now approve this pull request

Convert all the crates that have had their diagnostic migration
completed (except save_analysis because that will be deleted soon and
apfloat because of the licensing problem).
@Noratrieb
Copy link
Member Author

@bors r=jackh726

Btw, I have bors permissions already, no need to delegate :D

@bors
Copy link
Contributor

bors commented Jan 5, 2023

📌 Commit fd7a159 has been approved by jackh726

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 5, 2023
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 5, 2023
@bors
Copy link
Contributor

bors commented Jan 6, 2023

⌛ Testing commit fd7a159 with merge 0853d96...

@bors
Copy link
Contributor

bors commented Jan 6, 2023

☀️ Test successful - checks-actions
Approved by: jackh726
Pushing 0853d96 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 6, 2023
@bors bors merged commit 0853d96 into rust-lang:master Jan 6, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 6, 2023
@Noratrieb Noratrieb deleted the inline-fmt-part-1 branch January 6, 2023 06:11
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (0853d96): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.3%, -0.3%] 1
All ❌✅ (primary) - - 0

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.0% [2.0%, 2.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic merged-by-bors This PR was explicitly merged by bors. 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
Development

Successfully merging this pull request may close these issues.

7 participants