Rollup merge of #121739 - jieyouxu:loooong-typename, r=estebank

Display short types for unimplemented trait Shortens unimplemented trait diagnostics. Now shows: ``` error[E0277]: `Option<Option<Option<...>>>` doesn't implement `std::fmt::Display` --> $DIR/on_unimplemented_long_types.rs:4:17 | LL | pub fn foo() -> impl std::fmt::Display { | ^^^^^^^^^^^^^^^^^^^^^^ `Option<Option<Option<...>>>` cannot be formatted with the default formatter LL | LL | / Some(Some(Some(Some(Some(Some(Some(Some(Some(S... LL | | Some(Some(Some(Some(Some(Some(Some(Some(So... LL | | Some(Some(Some(Some(Some(Some(Some(Som... LL | | Some(Some(Some(Some(Some(Some(Some... ... | LL | | ))))))))))), LL | | ))))))))))) | |_______________- return type was inferred to be `Option<Option<Option<...>>>` here | = help: the trait `std::fmt::Display` is not implemented for `Option<Option<Option<...>>>` = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead error: aborting due to 1 previous error For more information about this error, try `rustc --explain E0277`. ``` I'm not 100% sure if this is desirable, or if we should just let the long types remain long. This is also kinda a short-term bandaid solution. The real long term solution is to properly migrate `rustc_trait_selection`'s error reporting to use translatable diagnostics and then properly handle type name printing. Fixes #121687.
rust-lang · Mar 2, 2024 · 3432d1b · 3432d1b
2 parents b2c933a + 62baa67
commit 3432d1b
Show file tree

Hide file tree

Showing 6 changed files with 117 additions and 50 deletions.
diff --git a/compiler/rustc_hir_typeck/src/method/suggest.rs b/compiler/rustc_hir_typeck/src/method/suggest.rs
@@ -1049,6 +1049,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     bound_list.into_iter().map(|(_, path)| path).collect::<Vec<_>>().join("\n");
                 let actual_prefix = rcvr_ty.prefix_string(self.tcx);
                 info!("unimplemented_traits.len() == {}", unimplemented_traits.len());
+                let mut long_ty_file = None;
                 let (primary_message, label) = if unimplemented_traits.len() == 1
                     && unimplemented_traits_only
                 {
@@ -1061,8 +1062,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                                 // Avoid crashing.
                                 return (None, None);
                             }
-                            let OnUnimplementedNote { message, label, .. } =
-                                self.err_ctxt().on_unimplemented_note(trait_ref, &obligation);
+                            let OnUnimplementedNote { message, label, .. } = self
+                                .err_ctxt()
+                                .on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file);
                             (message, label)
                         })
                         .unwrap()
@@ -1076,6 +1078,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                     )
                 });
                 err.primary_message(primary_message);
+                if let Some(file) = long_ty_file {
+                    err.note(format!(
+                        "the full name for the type has been written to '{}'",
+                        file.display(),
+                    ));
+                    err.note(
+                        "consider using `--verbose` to print the full type name to the console",
+                    );
+                }
                 if let Some(label) = label {
                     custom_span_label = true;
                     err.span_label(span, label);

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
@@ -16,6 +16,7 @@ use rustc_session::lint::builtin::UNKNOWN_OR_MALFORMED_DIAGNOSTIC_ATTRIBUTES;
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::Span;
 use std::iter;
+use std::path::PathBuf;
 
 use crate::errors::{
     EmptyOnClauseInOnUnimplemented, InvalidOnClauseInOnUnimplemented, NoValueInOnUnimplemented,
@@ -110,6 +111,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         &self,
         trait_ref: ty::PolyTraitRef<'tcx>,
         obligation: &PredicateObligation<'tcx>,
+        long_ty_file: &mut Option<PathBuf>,
     ) -> OnUnimplementedNote {
         let (def_id, args) = self
             .impl_similar_to(trait_ref, obligation)
@@ -265,7 +267,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
         }));
 
         if let Ok(Some(command)) = OnUnimplementedDirective::of_item(self.tcx, def_id) {
-            command.evaluate(self.tcx, trait_ref, &flags)
+            command.evaluate(self.tcx, trait_ref, &flags, long_ty_file)
         } else {
             OnUnimplementedNote::default()
         }
@@ -657,6 +659,7 @@ impl<'tcx> OnUnimplementedDirective {
         tcx: TyCtxt<'tcx>,
         trait_ref: ty::TraitRef<'tcx>,
         options: &[(Symbol, Option<String>)],
+        long_ty_file: &mut Option<PathBuf>,
     ) -> OnUnimplementedNote {
         let mut message = None;
         let mut label = None;
@@ -669,6 +672,7 @@ impl<'tcx> OnUnimplementedDirective {
             options.iter().filter_map(|(k, v)| v.clone().map(|v| (*k, v))).collect();
 
         for command in self.subcommands.iter().chain(Some(self)).rev() {
+            debug!(?command);
             if let Some(ref condition) = command.condition
                 && !attr::eval_condition(condition, &tcx.sess, Some(tcx.features()), &mut |cfg| {
                     let value = cfg.value.map(|v| {
@@ -680,7 +684,12 @@ impl<'tcx> OnUnimplementedDirective {
                                 span: cfg.span,
                                 is_diagnostic_namespace_variant: false
                             }
-                            .format(tcx, trait_ref, &options_map)
+                            .format(
+                                tcx,
+                                trait_ref,
+                                &options_map,
+                                long_ty_file
+                            )
                         )
                     });
 
@@ -709,10 +718,14 @@ impl<'tcx> OnUnimplementedDirective {
         }
 
         OnUnimplementedNote {
-            label: label.map(|l| l.format(tcx, trait_ref, &options_map)),
-            message: message.map(|m| m.format(tcx, trait_ref, &options_map)),
-            notes: notes.into_iter().map(|n| n.format(tcx, trait_ref, &options_map)).collect(),
-            parent_label: parent_label.map(|e_s| e_s.format(tcx, trait_ref, &options_map)),
+            label: label.map(|l| l.format(tcx, trait_ref, &options_map, long_ty_file)),
+            message: message.map(|m| m.format(tcx, trait_ref, &options_map, long_ty_file)),
+            notes: notes
+                .into_iter()
+                .map(|n| n.format(tcx, trait_ref, &options_map, long_ty_file))
+                .collect(),
+            parent_label: parent_label
+                .map(|e_s| e_s.format(tcx, trait_ref, &options_map, long_ty_file)),
             append_const_msg,
         }
     }
@@ -814,6 +827,7 @@ impl<'tcx> OnUnimplementedFormatString {
         tcx: TyCtxt<'tcx>,
         trait_ref: ty::TraitRef<'tcx>,
         options: &FxHashMap<Symbol, String>,
+        long_ty_file: &mut Option<PathBuf>,
     ) -> String {
         let name = tcx.item_name(trait_ref.def_id);
         let trait_str = tcx.def_path_str(trait_ref.def_id);
@@ -824,7 +838,11 @@ impl<'tcx> OnUnimplementedFormatString {
             .filter_map(|param| {
                 let value = match param.kind {
                     GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
-                        trait_ref.args[param.index as usize].to_string()
+                        if let Some(ty) = trait_ref.args[param.index as usize].as_type() {
+                            tcx.short_ty_string(ty, long_ty_file)
+                        } else {
+                            trait_ref.args[param.index as usize].to_string()
+                        }
                     }
                     GenericParamDefKind::Lifetime => return None,
                 };

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/suggestions.rs
@@ -2671,6 +2671,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
     ) where
         T: ToPredicate<'tcx>,
     {
+        let mut long_ty_file = None;
+
         let tcx = self.tcx;
         let predicate = predicate.to_predicate(tcx);
         match *cause_code {
@@ -2853,21 +2855,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 }
             }
             ObligationCauseCode::Coercion { source, target } => {
-                let mut file = None;
-                let source = tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut file);
-                let target = tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut file);
+                let source =
+                    tcx.short_ty_string(self.resolve_vars_if_possible(source), &mut long_ty_file);
+                let target =
+                    tcx.short_ty_string(self.resolve_vars_if_possible(target), &mut long_ty_file);
                 err.note(with_forced_trimmed_paths!(format!(
                     "required for the cast from `{source}` to `{target}`",
                 )));
-                if let Some(file) = file {
-                    err.note(format!(
-                        "the full name for the type has been written to '{}'",
-                        file.display(),
-                    ));
-                    err.note(
-                        "consider using `--verbose` to print the full type name to the console",
-                    );
-                }
             }
             ObligationCauseCode::RepeatElementCopy {
                 is_constable,
@@ -3170,8 +3164,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 // Don't print the tuple of capture types
                 'print: {
                     if !is_upvar_tys_infer_tuple {
-                        let mut file = None;
-                        let ty_str = tcx.short_ty_string(ty, &mut file);
+                        let ty_str = tcx.short_ty_string(ty, &mut long_ty_file);
                         let msg = format!("required because it appears within the type `{ty_str}`");
                         match ty.kind() {
                             ty::Adt(def, _) => match tcx.opt_item_ident(def.did()) {
@@ -3269,9 +3262,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 let mut parent_trait_pred =
                     self.resolve_vars_if_possible(data.derived.parent_trait_pred);
                 let parent_def_id = parent_trait_pred.def_id();
-                let mut file = None;
-                let self_ty_str =
-                    tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file);
+                let self_ty_str = tcx
+                    .short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut long_ty_file);
                 let trait_name = parent_trait_pred.print_modifiers_and_trait_path().to_string();
                 let msg = format!("required for `{self_ty_str}` to implement `{trait_name}`");
                 let mut is_auto_trait = false;
@@ -3329,15 +3321,6 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     }
                 };
 
-                if let Some(file) = file {
-                    err.note(format!(
-                        "the full type name has been written to '{}'",
-                        file.display(),
-                    ));
-                    err.note(
-                        "consider using `--verbose` to print the full type name to the console",
-                    );
-                }
                 let mut parent_predicate = parent_trait_pred;
                 let mut data = &data.derived;
                 let mut count = 0;
@@ -3378,22 +3361,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                         count,
                         pluralize!(count)
                     ));
-                    let mut file = None;
-                    let self_ty =
-                        tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty(), &mut file);
+                    let self_ty = tcx.short_ty_string(
+                        parent_trait_pred.skip_binder().self_ty(),
+                        &mut long_ty_file,
+                    );
                     err.note(format!(
                         "required for `{self_ty}` to implement `{}`",
                         parent_trait_pred.print_modifiers_and_trait_path()
                     ));
-                    if let Some(file) = file {
-                        err.note(format!(
-                            "the full type name has been written to '{}'",
-                            file.display(),
-                        ));
-                        err.note(
-                            "consider using `--verbose` to print the full type name to the console",
-                        );
-                    }
                 }
                 // #74711: avoid a stack overflow
                 ensure_sufficient_stack(|| {
@@ -3502,7 +3477,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
             }
             ObligationCauseCode::OpaqueReturnType(expr_info) => {
                 if let Some((expr_ty, expr_span)) = expr_info {
-                    let expr_ty = with_forced_trimmed_paths!(self.ty_to_string(expr_ty));
+                    let expr_ty = self.tcx.short_ty_string(expr_ty, &mut long_ty_file);
                     err.span_label(
                         expr_span,
                         with_forced_trimmed_paths!(format!(
@@ -3512,6 +3487,14 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                 }
             }
         }
+
+        if let Some(file) = long_ty_file {
+            err.note(format!(
+                "the full name for the type has been written to '{}'",
+                file.display(),
+            ));
+            err.note("consider using `--verbose` to print the full type name to the console");
+        }
     }
 
     #[instrument(

diff --git a/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs b/compiler/rustc_trait_selection/src/traits/error_reporting/type_err_ctxt_ext.rs
@@ -395,6 +395,7 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                     kind: _,
                 } = *obligation.cause.code()
                 {
+                    debug!("ObligationCauseCode::CompareImplItemObligation");
                     return self.report_extra_impl_obligation(
                         span,
                         impl_item_def_id,
@@ -445,18 +446,21 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                                 )
                             })
                             .unwrap_or_default();
-                        let file_note = file.map(|file| format!(
+                        let file_note = file.as_ref().map(|file| format!(
                             "the full trait has been written to '{}'",
                             file.display(),
                         ));
 
+                        let mut long_ty_file = None;
+
                         let OnUnimplementedNote {
                             message,
                             label,
                             notes,
                             parent_label,
                             append_const_msg,
-                        } = self.on_unimplemented_note(trait_ref, &obligation);
+                        } = self.on_unimplemented_note(trait_ref, &obligation, &mut long_ty_file);
+
                         let have_alt_message = message.is_some() || label.is_some();
                         let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id());
                         let is_unsize =
@@ -511,6 +515,13 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
 
                         let mut err = struct_span_code_err!(self.dcx(), span, E0277, "{}", err_msg);
 
+                        if let Some(long_ty_file) = long_ty_file {
+                            err.note(format!(
+                                "the full name for the type has been written to '{}'",
+                                long_ty_file.display(),
+                            ));
+                            err.note("consider using `--verbose` to print the full type name to the console");
+                        }
                         let mut suggested = false;
                         if is_try_conversion {
                             suggested = self.try_conversion_context(&obligation, trait_ref.skip_binder(), &mut err);
@@ -758,6 +769,8 @@ impl<'tcx> TypeErrCtxt<'_, 'tcx> {
                             return err.emit();
                         }
 
+
+
                         err
                     }
 

diff --git a/tests/ui/traits/on_unimplemented_long_types.rs b/tests/ui/traits/on_unimplemented_long_types.rs
@@ -0,0 +1,17 @@
+//@ compile-flags: --diagnostic-width=60 -Z write-long-types-to-disk=yes
+//@ normalize-stderr-test: "long-type-\d+" -> "long-type-hash"
+
+pub fn foo() -> impl std::fmt::Display {
+    //~^ ERROR doesn't implement `std::fmt::Display`
+    Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(
+        Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(
+            Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(
+                Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(Some(
+                    Some(Some(Some(Some(Some(Some(Some(Some(())))))))),
+                ))))))))))),
+            ))))))))))),
+        ))))))))))),
+    )))))))))))
+}
+
+fn main() {}
diff --git a/tests/ui/traits/on_unimplemented_long_types.stderr b/tests/ui/traits/on_unimplemented_long_types.stderr
@@ -0,0 +1,25 @@
+error[E0277]: `Option<Option<Option<...>>>` doesn't implement `std::fmt::Display`
+  --> $DIR/on_unimplemented_long_types.rs:4:17
+   |
+LL |   pub fn foo() -> impl std::fmt::Display {
+   |                   ^^^^^^^^^^^^^^^^^^^^^^ `Option<Option<Option<...>>>` cannot be formatted with the default formatter
+LL |
+LL | /     Some(Some(Some(Some(Some(Some(Some(Some(Some(S...
+LL | |         Some(Some(Some(Some(Some(Some(Some(Some(So...
+LL | |             Some(Some(Some(Some(Some(Some(Some(Som...
+LL | |                 Some(Some(Some(Some(Some(Some(Some...
+...  |
+LL | |         ))))))))))),
+LL | |     )))))))))))
+   | |_______________- return type was inferred to be `Option<Option<Option<...>>>` here
+   |
+   = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt'
+   = note: consider using `--verbose` to print the full type name to the console
+   = help: the trait `std::fmt::Display` is not implemented for `Option<Option<Option<...>>>`
+   = note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
+   = note: the full name for the type has been written to '$TEST_BUILD_DIR/traits/on_unimplemented_long_types/on_unimplemented_long_types.long-type-hash.txt'
+   = note: consider using `--verbose` to print the full type name to the console
+
+error: aborting due to 1 previous error
+
+For more information about this error, try `rustc --explain E0277`.