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

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>`

`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. On E0599 show type identity to avoid expanding the receiver's generic parameters. Unify wording on `long_ty_path` everywhere.

fee1-dead · 2025-08-08T04:53:09Z

@bors r+

bors · 2025-08-08T04:53:13Z

📌 Commit 9919665 has been approved by fee1-dead

It is now in the queue for this repository.

Use `tcx.short_string()` in more diagnostics `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).

Rollup of 7 pull requests Successful merges: - #144039 (Use `tcx.short_string()` in more diagnostics) - #144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - #144823 (coverage: Extract HIR-related helper code out of the main module) - #144987 (Enable f16 and f128 on targets that were fixed in LLVM21) - #145001 (regression test for intrinsics may not inline properly on pclmulqdq) - #145080 (Escape diff strings in MIR dataflow graphviz) - #145083 (Fix cross-compilation of Cargo) r? `@ghost` `@rustbot` modify labels: rollup

joshtriplett · 2025-08-08T20:04:32Z

Do the modified tests still test the exact value of the long type name that's written to the separate file? I think they should.

Rollup of 8 pull requests Successful merges: - #139451 (Add `target_env = "macabi"` and `target_env = "sim"`) - #144039 (Use `tcx.short_string()` in more diagnostics) - #144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - #144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses) - #144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697)) - #144649 (Account for bare tuples and `Pin` methods in field searching logic) - #144775 (more strongly dissuade use of `skip_binder`) - #144987 (Enable f16 and f128 on targets that were fixed in LLVM21) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #144039 - estebank:short-paths, r=fee1-dead Use `tcx.short_string()` in more diagnostics `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).

Rollup of 8 pull requests Successful merges: - rust-lang/rust#139451 (Add `target_env = "macabi"` and `target_env = "sim"`) - rust-lang/rust#144039 (Use `tcx.short_string()` in more diagnostics) - rust-lang/rust#144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - rust-lang/rust#144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses) - rust-lang/rust#144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697)) - rust-lang/rust#144649 (Account for bare tuples and `Pin` methods in field searching logic) - rust-lang/rust#144775 (more strongly dissuade use of `skip_binder`) - rust-lang/rust#144987 (Enable f16 and f128 on targets that were fixed in LLVM21) r? `@ghost` `@rustbot` modify labels: rollup

estebank · 2025-08-10T18:06:59Z

Do the modified tests still test the exact value of the long type name that's written to the separate file?

@joshtriplett We don't do that at the moment. We'd have to teach the test runner to look into them. Shouldn't be too hard. Another option is to make these tests have two revisions, one writing to disk and one using the default, that way we can see the full type in the stderr file.

Rollup of 8 pull requests Successful merges: - rust-lang#139451 (Add `target_env = "macabi"` and `target_env = "sim"`) - rust-lang#144039 (Use `tcx.short_string()` in more diagnostics) - rust-lang#144192 (atomicrmw on pointers: move integer-pointer cast hacks into backend) - rust-lang#144545 (In rustc_pattern_analysis, put `true` witnesses before `false` witnesses) - rust-lang#144579 (Implement declarative (`macro_rules!`) attribute macros (RFC 3697)) - rust-lang#144649 (Account for bare tuples and `Pin` methods in field searching logic) - rust-lang#144775 (more strongly dissuade use of `skip_binder`) - rust-lang#144987 (Enable f16 and f128 on targets that were fixed in LLVM21) r? `@ghost` `@rustbot` modify labels: rollup

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 3 times, most recently from 6397fa3 to 5963f7a Compare July 30, 2025 16:50

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

estebank force-pushed the short-paths branch from e1128f8 to df19abd Compare August 6, 2025 22:30

This comment has been minimized.

Sign in to view

estebank force-pushed the short-paths branch from df19abd to 371b481 Compare August 7, 2025 19:08

This comment has been minimized.

Sign in to view

estebank force-pushed the short-paths branch from 371b481 to 9919665 Compare August 7, 2025 21:18

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 Aug 8, 2025

tgross35 mentioned this pull request Aug 8, 2025

Rollup of 7 pull requests #145090

Closed

tgross35 mentioned this pull request Aug 8, 2025

Rollup of 8 pull requests #145126

Merged

bors merged commit f5dda19 into rust-lang:master Aug 9, 2025
10 checks passed

rustbot added this to the 1.91.0 milestone Aug 9, 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

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.

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!

This comment has been minimized.

This comment has been minimized.

fee1-dead commented Aug 8, 2025

Uh oh!

bors commented Aug 8, 2025

Uh oh!

joshtriplett commented Aug 8, 2025

Uh oh!

Uh oh!

estebank commented Aug 10, 2025

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