Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always#118268

compiler-errors · 2023-11-24T22:45:50Z

It's almost always better, at least in diagnostics, to print Fn(i32, u32) instead of Fn<(i32, u32)>.

Related to but doesn't fix #118225. That needs a separate fix.

rustbot · 2023-11-24T22:45:58Z

r? @wesleywiser

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

rustbot · 2023-11-24T22:46:00Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler-errors · 2023-11-24T22:47:09Z

src/tools/clippy/clippy_lints/src/future_not_send.rs

                                        "`{}` doesn't implement `{}`",
                                        trait_pred.self_ty(),
-                                        trait_pred.trait_ref.print_only_trait_path(),
+                                        trait_pred.trait_ref.print_trait_sugared(),


I guess this doesn't matter -- can revert. I actually don't even know why this uses print_only_trait_path, given that it's always Send.

It's likely copy-pasted from a more general rustc error.

estebank · 2023-11-28T18:20:09Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

-                            "ambiguous `{assoc_name}` from `{}`",
-                            bound.print_only_trait_path(),
-                        ),
+                        format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),


Suggested change

format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),

format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared()),

We have a lot of these :(

I ctrl+f'd for ,); and ,), yesterday and found like >50 instances.

compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs

estebank · 2023-11-28T18:26:15Z

compiler/rustc_middle/src/ty/print/pretty.rs

+    TraitRefPrintSugared<'tcx> {
+        if !with_no_queries()
+            && let Some(kind) = cx.tcx().fn_trait_kind_from_def_id(self.0.def_id)
+            && let ty::Tuple(args) = self.0.args.type_at(1).kind()
+        {
+            p!(write("{}", kind.as_str()), "(");
+            for (i, arg) in args.iter().enumerate() {
+                if i > 0 {
+                    p!(", ");
+                }
+                p!(print(arg));
+            }
+            p!(")");
+        } else {
+            p!(print_def_path(self.0.def_id, self.0.args));
+        }
+    }


This will never print the Output, is that ok?

There's no Output that we have access to, unfortunately :/

Didn't we hack something together for Future::Output somewhere?

Edit: I think I was thinking of pretty_print_opaque_impl_type.

I guess I could eagerly look into the self type and see if it's something with a signature, but it's not always going to be possible. Let me see.

If you manage it, great. If not, r=me.

Actually, when we extract this information from the self type, it's often not right. For example, when we have:

fn test<F: Fn(u32) -> u32>() {} test(|_: i32| -> i32 { 0 });

Extracting the return type from the self type will give us an error message mentioning something like {closure}: Fn(u32) -> i32, not Fn(u32) -> u32.

compiler/rustc_middle/src/ty/print/pretty.rs

compiler-errors · 2023-11-28T22:09:55Z

@bors r=estebank

bors · 2023-11-28T22:09:57Z

📌 Commit ecccf33 has been approved by estebank

It is now in the queue for this repository.

…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#118157 (Add `never_patterns` feature gate) - rust-lang#118191 (Suggest `let` or `==` on typo'd let-chain) - rust-lang#118231 (also add is_empty to const raw slices) - rust-lang#118333 (Print list of missing target features when calling a function with target features outside an unsafe block) - rust-lang#118426 (ConstProp: Correctly remove const if unknown value assigned to it.) - rust-lang#118428 (rustdoc: Move `AssocItemRender` and `RenderMode` to `html::render`.) - rust-lang#118438 (Update nto-qnx.md) Failed merges: - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2023-11-29T16:31:09Z

🔒 Merge conflict

This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again.

How do I rebase?

Assuming self is your fork and upstream is this repository, you can resolve the conflict following these steps:

git checkout pretty-print (switch to your branch)
git fetch upstream master (retrieve the latest master)
git rebase upstream/master -p (rebase on top of it)
Follow the on-screen instruction to resolve conflicts (check git status if you got lost).
git push self pretty-print --force-with-lease (update this PR)

You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial.

Please avoid the "Resolve conflicts" button on GitHub. It uses git merge instead of git rebase which makes the PR commit history more difficult to read.

Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Cargo.lock conflict is handled during merge and rebase. This is normal, and you should still perform step 5 to update this PR.

Error message

Auto-merging compiler/rustc_middle/src/ty/print/pretty.rs
Auto-merging compiler/rustc_infer/src/infer/error_reporting/mod.rs
CONFLICT (content): Merge conflict in compiler/rustc_infer/src/infer/error_reporting/mod.rs
Auto-merging compiler/rustc_hir_analysis/src/astconv/mod.rs
Automatic merge failed; fix conflicts and then commit the result.

compiler-errors · 2023-12-05T17:16:18Z

@bors r=estebank

bors · 2023-12-05T17:16:21Z

📌 Commit f6c30b3 has been approved by estebank

It is now in the queue for this repository.

…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.

…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#118268 - compiler-errors:pretty-print, r=estebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.

fmease · 2023-12-10T12:55:33Z

Actually, I wonder if we should print Fn<(…,)> as Fn(…,) -> _ instead of Fn(…,) since that's closer to its actual meaning. The latter (which this PR has implemented) looks like Fn<(…,), Output = ()> to me.

rustbot assigned wesleywiser Nov 24, 2023

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 Nov 24, 2023

compiler-errors commented Nov 24, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

rust-cloud-vms bot force-pushed the pretty-print branch from ef938e9 to ecccf33 Compare November 27, 2023 03:33

estebank reviewed Nov 28, 2023

View reviewed changes

compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs Show resolved Hide resolved

estebank reviewed Nov 28, 2023

View reviewed changes

compiler/rustc_middle/src/ty/print/pretty.rs Show resolved Hide resolved

estebank reviewed Nov 28, 2023

View reviewed changes

compiler/rustc_middle/src/ty/print/pretty.rs Show resolved Hide resolved

estebank approved these changes Nov 28, 2023

View reviewed changes

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 Nov 28, 2023

This was referenced Nov 29, 2023

Rollup of 6 pull requests #118432

Closed

Rollup of 6 pull requests #118435

Closed

matthiaskrgr mentioned this pull request Nov 29, 2023

Rollup of 7 pull requests #118443

Merged

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 29, 2023

compiler-errors added 2 commits December 5, 2023 17:15

Add print_trait_sugared

b97ff8e

Add more

f6c30b3

rust-cloud-vms bot force-pushed the pretty-print branch from ecccf33 to f6c30b3 Compare December 5, 2023 17:16

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 5, 2023

matthiaskrgr mentioned this pull request Dec 5, 2023

Rollup of 7 pull requests #118651

Closed

compiler-errors mentioned this pull request Dec 5, 2023

Rollup of 8 pull requests #118652

Closed

This was referenced Dec 5, 2023

Rollup of 8 pull requests #118653

Closed

Rollup of 9 pull requests #118655

Merged

bors merged commit ad23f30 into rust-lang:master Dec 6, 2023

rustbot added this to the 1.76.0 milestone Dec 6, 2023

	format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),),
	format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared()),

Uh oh!

Conversation

compiler-errors commented Nov 24, 2023

Uh oh!

rustbot commented Nov 24, 2023

Uh oh!

rustbot commented Nov 24, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

estebank Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

compiler-errors commented Nov 28, 2023

Uh oh!

bors commented Nov 28, 2023

Uh oh!

bors commented Nov 29, 2023

Uh oh!

compiler-errors commented Dec 5, 2023

Uh oh!

bors commented Dec 5, 2023

Uh oh!

fmease commented Dec 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

estebank Nov 28, 2023 •

edited

Loading

compiler-errors Nov 28, 2023 •

edited

Loading

fmease commented Dec 10, 2023 •

edited

Loading