Do not show ::{{constructor}} on tuple struct diagnostics

[estebank]

Fix #41313.

[rust-highfive]

r? @nikomatsakis

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

[shepmaster]

Thank you, @estebank! We'll check in every so often to make sure that @nikomatsakis or another reviewer gets back to you soon!

[nikomatsakis]

Hmm, wait, actually -- this same code is also used for generating symbol names and the like. @michaelwoerister can you think of any complications that might arise from changing this?

[michaelwoerister]

This should be fine. Symbol names are fully distinguished by the hash value at the end.

[michaelwoerister]

@bors r+

[bors]

📌 Commit cd60307 has been approved by michaelwoerister

[bors]

⌛ Testing commit cd60307 with merge bbe4dc2...

[petrochenkov]

This PR is masking a deeper problem - why is a constructor DefId used in context where struct's DefId is supposed to be used? How many other contexts are affected in the same way?

I think the underlying problem need to be fixed and ::{{constructor}} should be kept for actual constructors.
@bors r-

[bors]

💔 Test failed - status-appveyor

[Mark-Simulacrum]

Just noting that the appveyor build here failed with #40546, but not retrying because of the r- above.

[nikomatsakis]

@petrochenkov perhaps so. We do sometimes use the constructor def-id for tuple structs, I think? But it's been a while since I looked at this code. Worth investigating.

@estebank is it easy for you to get a RUST_BACKTRACE=1 where the error occurs? (You may find -Ztreat-err-as-bug helpful.) That would help pinpoint where the error is being reported.

[estebank]

@nikomatsakis @petrochenkov I believe this is caused because of these lines in collect.rs:

        ItemStruct(ref def, _) => {
            // Use separate constructor id for unit/tuple structs and reuse did for braced structs.
            let ctor_id = if !def.is_struct() {
                Some(tcx.hir.local_def_id(def.id()))
            } else {
                None
            };
            (AdtKind::Struct, vec![
                convert_struct_variant(tcx, ctor_id.unwrap_or(def_id), item.name,
                                       ty::VariantDiscr::Relative(0), def)
            ])
        }

I'm intrigued by the comment as it says what it's doing, but it doesn't state why. It was originally incorporated in 2cdd9f1, I need to further read that commit to understand why.

[petrochenkov]

@estebank
I recall this now.
The situation with constructor DefIds is a bit of mess, and quite large refactoring is required to fix it. Structs and their constructors have different DefIds while variants have the same DefId for both variants themselves and constructors. Variants historically keep constructor ids if constructor exists and struct ids otherwise.
See #28816 (including "outdated" hidden comments) and #36814 (comment) for some background.

It was originally incorporated in 2cdd9f1

That commit only adds the comment, the DefId-assigning logic was pre-existing.

In the meantime, local fix can make diagnostics better:

diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
index 1086773041..450b0f95fc 100644
--- a/src/librustc_typeck/check/_match.rs
+++ b/src/librustc_typeck/check/_match.rs
@@ -660,8 +660,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                                etc: bool) {
         let tcx = self.tcx;

+        let mut diagn_did = variant.did;
         let (substs, kind_name) = match adt_ty.sty {
-            ty::TyAdt(adt, substs) => (substs, adt.variant_descr()),
+            ty::TyAdt(adt, substs) => {
+                if !adt.is_enum() && variant.ctor_kind != CtorKind::Fictive {
+                    diagn_did = tcx.parent_def_id(variant.did).expect("no struct def id");
+                }
+                (substs, adt.variant_descr())
+            }
             _ => span_bug!(span, "struct pattern is not an ADT")
         };

@@ -700,12 +706,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
                             struct_span_err!(tcx.sess, span, E0026,
                                              "{} `{}` does not have a field named `{}`",
                                              kind_name,
-                                             tcx.item_path_str(variant.did),
+                                             tcx.item_path_str(diagn_did),
                                              field.name)
                                 .span_label(span,
                                             &format!("{} `{}` does not have field `{}`",
                                                      kind_name,
-                                                     tcx.item_path_str(variant.did),
+                                                     tcx.item_path_str(diagn_did),
                                                      field.name))
                                 .emit();

[petrochenkov]

Actually, doesn't matter.
@bors r=michaelwoerister

[bors]

📌 Commit cd60307 has been approved by michaelwoerister

[arielb1]

@bors retry

[bors]

⌛ Testing commit cd60307 with merge 79b05fe...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing 79b05fe to master...