Use `tcx.short_string()` in more diagnostics #144039

estebank · 2025-07-16T19:45:42Z

TyCtxt::short_string ensures that user visible type paths aren't overwhelming on the terminal output, and properly saves the long name to disk as a side-channel. We already use these throughout the compiler and have been using them as needed when users find cases where the output is verbose. This is a proactive search of some cases to use short_string.

We add support for shortening the path of "trait path only".

Every manual use of short_string is a bright marker that that error should be using structured diagnostics instead (as they have proper handling of long types without the maintainer having to think abou tthem).

rustbot · 2025-07-16T19:45:46Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-07-16T19:45:48Z

HIR ty lowering was modified

cc @fmease

estebank · 2025-07-16T19:47:43Z

tests/ui/confuse-field-and-method/issue-2392.rs

+// FIXME(estebank): diagnostics with long type paths that don't print out the full path anywhere
+// still prints the note explaining where the type was written to.
+//@ compile-flags: -Zwrite-long-types-to-disk=yes


I found these by changing the default -Zwrite-long-types-to-disk=no in compiletest::runtest::TestCx::make_compile_args to yes, which highlighted some preexisting cases that I'll investigate.

One left: tests/ui/methods/filter-relevant-fn-bounds.rs. Also pre-existing.

Figured out what's happening on the last case standing: it's a consequence of using on_unimplemented. Given the way the machinery is set up, we have to create a map between the type parameter name of the annotated trait and the resolved type, which we need to shorten. Because this happens before we know which type parameters are even referenced in the annotation, we pessimize and write the types to disk when any of those params needs to be shortened, regardless of whether the corresponding message ever gets printed by the E0277. Changing this to use the full type, makes other diagnostics longer. The proper solution is to rearchitect on_unimplemented to make a map from type param to the actual type, and the shorten during rendering, instead of using strings throughout. I feel it's ok to leave that one case in the test suite be for now.

compiler/rustc_hir_typeck/src/method/suggest.rs

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

fee1-dead · 2025-07-21T16:00:45Z

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

-                    let ty_str = tcx.short_string(ty, err.long_ty_path());
-                    let msg = format!("required because it appears within the type `{ty_str}`");
+                    let mut msg = || {
+                        let ty_str = tcx.short_string(ty, err.long_ty_path());
+                        format!("required because it appears within the type `{ty_str}`")
+                    };


This probably illustrates a problem we have with the current system: we don't prevent if a short string was created but was never printed. I think there could be a mechanism (a drop guard for a custom short_string return newtype) in which we detect that. Could you open an issue?

fee1-dead · 2025-07-21T16:05:49Z

tests/ui/confuse-field-and-method/issue-18343.stderr

These ui tests are now regressions because we should not "show generic arguments when the method can't be found in any implementations". If you're trying to prevent us from creating a short string when we don't use it, maybe we should pass a closure down instead of the ty_str_reported.

I think that I originally made the message only mention the item name instead of the Ty<'_> was precisely because we didn't have short_string back then and we could end up with quite long output accidentally. I was concerned back then that there are cases where the method is available on a given type depending on bounds on its type parameters, so I felt comfortable relying exclusively on short_string for keeping the output readable.

#81576 says we shouldn't show generics at all if they are irrelevant. I agree. I think we should spend effort keeping that behavior here.

Would it be ok to display the type's identity (Option<T> instead of the current Option and this PR's Option<ElaboratedType>) or a freshened type (Option<_>)? I'd really want to keep the structured error's field a Ty<'_> instead of a String, and using the identity allows for it.

Last commit does this

fee1-dead · 2025-07-21T16:09:52Z

tests/ui/confuse-field-and-method/issue-2392.rs

inconsistent output in aarch64

seems quite odd. Could you explain what the issue is?

It was the error from #144039 (comment)

I suspect that that platform in particular prints a slightly different output for the closure type which puts it under the default terminal width threshold.

that platform in particular prints a slightly different output for the closure type

That is really really surprising for me. Could you post an issue for this? This seems very worthy of investigation.

compiler/rustc_hir_typeck/src/method/suggest.rs

fee1-dead · 2025-07-24T09:34:06Z

tests/ui/confuse-field-and-method/issue-18343.stderr

#81576 says we shouldn't show generics at all if they are irrelevant. I agree. I think we should spend effort keeping that behavior here.

rustbot · 2025-07-24T16:46:38Z

This PR modifies tests/ui/issues/. If this PR is adding new tests to tests/ui/issues/,
please refrain from doing so, and instead add it to more descriptive subdirectories.

compiler/rustc_hir_typeck/src/method/suggest.rs

rustbot · 2025-07-28T19:02:47Z

Some changes occurred in need_type_info.rs

cc @lcnr

`TyCtxt::short_string` ensures that user visible type paths aren't overwhelming on the terminal output, and properly saves the long name to disk as a side-channel. We already use these throughout the compiler and have been using them as needed when users find cases where the output is verbose. This is a proactive search of some cases to use `short_string`. We add support for shortening the path of "trait path only". Every manual use of `short_string` is a bright marker that that error should be using structured diagnostics instead (as they have proper handling of long types without the maintainer having to think abou tthem).

When we don't actually print out a shortened type we don't need the "use `--verbose`" note.

This test seems to have improper use of `TyCtxt::short_string`.

… parameters

fee1-dead

There's now even more things to review and I didn't finish reviewing this round. I recommend getting the instance changes into a separate (draft) PR which could land after this one.

I also anticipate the changes need to be squashed into a single commit.

compiler/rustc_middle/src/ty/error.rs

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

estebank · 2025-08-04T15:59:35Z

Removed the Instance code and moved it to #144914, which ended up being separate enough that the PR can land on its own (I could have sworn it was being built on top of functionality I was adding here, but it wasn't). Don't worry about the review slowness. I intended commits to be readable on their own and squashable at the end (so you wouldn't have to spend time differentiating between renames and functionality changes). I think your last (partial?) review got as far as the last commit that says "review comment".

fee1-dead

r=me after nit is applied and the commits are squashed

fee1-dead · 2025-08-05T05:04:35Z

tests/ui/methods/call_method_unknown_referent.rs

@@ -44,5 +44,5 @@ fn main() {
    // our resolution logic needs to be able to call methods such as foo()
    // on the outer type even if the inner type is ambiguous.
    let _c = (ptr as SmartPtr<_>).read();
-    //~^ ERROR no method named `read` found for struct `SmartPtr`
+    //~^ ERROR no method named `read` found for struct `SmartPtr


Suggested change

//~^ ERROR no method named `read` found for struct `SmartPtr

//~^ ERROR no method named `read` found for struct `SmartPtr<T>`

rustbot assigned fee1-dead Jul 16, 2025

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 Jul 16, 2025

estebank commented Jul 16, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

estebank force-pushed the short-paths branch from 38a3323 to dc45f5f Compare July 18, 2025 16:59

estebank commented Jul 18, 2025

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

fee1-dead requested changes Jul 21, 2025

View reviewed changes

rustbot 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 Jul 21, 2025

estebank force-pushed the short-paths branch from dc45f5f to 0786a11 Compare July 21, 2025 20:16

estebank mentioned this pull request Jul 23, 2025

Devise mechanism for ensuring tcx.short_string is actually used #144374

Open

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 23, 2025

fee1-dead reviewed Jul 24, 2025

View reviewed changes

fee1-dead requested changes Jul 26, 2025

View reviewed changes

compiler/rustc_hir_typeck/src/method/suggest.rs Outdated Show resolved Hide resolved

rustbot 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 Jul 26, 2025

estebank force-pushed the short-paths branch from 512f5eb to 003b911 Compare July 28, 2025 02:37

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 28, 2025

This comment has been minimized.

Sign in to view

estebank force-pushed the short-paths branch from 7da892f to d72d47a Compare July 29, 2025 02:07

estebank added 2 commits July 29, 2025 15:46

Remove useless note

7b79e47

When we don't actually print out a shortened type we don't need the "use `--verbose`" note.

estebank added 9 commits July 29, 2025 15:56

Tweak use of short_string in method errors

6404466

Mark test to use short type paths

33d65fc

This test seems to have improper use of `TyCtxt::short_string`.

Minor fixes of short_string handling

35e31fc

Revert change to one test because of inconsistent output in aarc64

98542e7

review comment: rename err_msg closure to default_err_msg

d22f77d

On E0599 show type identity to avoid expanding the receiver's generic…

70f21b6

… parameters

Unify wording on long_ty_path everywhere

2de4cd8

remove unnecessary code

4942e38

Remove redundant code and rename bindings for consistency

e1128f8

estebank force-pushed the short-paths branch 2 times, most recently from 6397fa3 to 5963f7a Compare July 30, 2025 16:50

fee1-dead requested changes Aug 2, 2025

View reviewed changes

compiler/rustc_middle/src/ty/error.rs Outdated Show resolved Hide resolved

compiler/rustc_middle/src/ty/print/mod.rs Outdated Show resolved Hide resolved

rustbot 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 Aug 2, 2025

estebank force-pushed the short-paths branch from 45e782d to e1128f8 Compare August 4, 2025 15:47

estebank added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 4, 2025

fee1-dead requested changes Aug 5, 2025

View reviewed changes

rustbot 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 Aug 5, 2025

	//~^ ERROR no method named `read` found for struct `SmartPtr
	//~^ ERROR no method named `read` found for struct `SmartPtr<T>`

Use tcx.short_string() in more diagnostics #144039

Are you sure you want to change the base?

Use tcx.short_string() in more diagnostics #144039

Conversation

estebank commented Jul 16, 2025

Uh oh!

rustbot commented Jul 16, 2025

Uh oh!

rustbot commented Jul 16, 2025

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!

This comment has been minimized.

This comment has been minimized.

Uh oh!

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!

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!

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!

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!

rustbot commented Jul 24, 2025

Uh oh!

Uh oh!

rustbot commented Jul 28, 2025

Uh oh!

This comment has been minimized.

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

estebank commented Aug 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fee1-dead left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Use `tcx.short_string()` in more diagnostics #144039

Use `tcx.short_string()` in more diagnostics #144039

estebank commented Aug 4, 2025 •

edited

Loading