Give better diagnostic when using a private tuple struct constructor

[guswynn]

Fixes #75907

Some notes:

1. This required some deep changes, including removing a Copy impl for PatKind. If some tests fail, I would still appreciate review on the overall approach
2. this only works with basic patterns (no wildcards for example), and fails if there is any problems getting the visibility of the fields (i am not sure what the failure that can happen in resolve_visibility_speculative, but we check the length of our fields in both cases against each other, so if anything goes wrong, we fall back to the worse error. This could be extended to more patterns
3. this does not yet deal with [diagnostics] Attempting to initialize a private field in a public tuple struct yields confusing error message #75906, but I believe it will be similar
4. let me know if you want more tests
5. doesn't yet at the suggestion that @yoshuawuyts suggested at the end of their issue, but that could be added relatively easily (i believe)

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[jyn514]

I'd be interested whether this affects perf.

[guswynn]

@jyn514 I think the clones are the same as the original Copy's, but not sure, is there a way to queue a perf check?

[jyn514]

Yes, but I'd prefer not to until someone familiar with this part of the compiler agrees this is the right approach (the perf server has a tendency to fall behind master if you queue too many builds).

[Aaron1011]

There's currently only one job in the perf queue, so I think it should be fine. Also, the total perf job running time was decreased last month in rust-lang/rustc-perf#740, and I haven't seen the perf queue growing out of control since them.

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit b1075c6eae2da6f043d333bf99985120850efda0 with merge 1e1632e45f7c9ad03e2e1c4a32e81de6019cf26f...

[jyn514]

Oh nice, I didn't realize perf was no longer running script-servo :)

[bors]

☀️ Try build successful - checks-actions, checks-azure
Build commit: 1e1632e45f7c9ad03e2e1c4a32e81de6019cf26f (1e1632e45f7c9ad03e2e1c4a32e81de6019cf26f)

[rust-timer]

Queued 1e1632e45f7c9ad03e2e1c4a32e81de6019cf26f with parent 90782cb, future comparison URL.

[rust-timer]

Finished benchmarking try commit (1e1632e45f7c9ad03e2e1c4a32e81de6019cf26f): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never

[jyn514]

Perf results look like noise, it says max-rss went down but since you added more fields here that can't be right.

[petrochenkov]

r? @petrochenkov

[petrochenkov]

This PR hides the correct error that attempts to teach you the actual language rules ("private fields make the constructor private") and masks it with something plausibly looking, but not really correct.

[petrochenkov]

It would be better to keep the current primary message and add a note to it with a multi-span pointing to those private fields that it mentions.

[guswynn]

Genuinely astounded that adding 'a: 'ast bounds in 3 places resolved the errors i got, that said, i suppose nothing lives longer than the ast

[guswynn]

@estebank do you think it will be not too noisy to add a span_label for every private field? Other than question, your idea to make a multispan from the span's, label them, add a note, and set the span and then instead of set_primary_message, use span_note

[petrochenkov]

Suggested change

	let mut ret_fields = vec![];
	let mut field_visibilities = Vec::with_capacity(vdata.fields().len());

[petrochenkov]

Not addressed yet.

[guswynn]

This doesn't work cross-crate :((( I added a test that shows that

Not sure what I should do to resolve that

[petrochenkov]

I'll try to check what can be done in the cross-crate case, but it's better done in a separate PR.
We should be able to decode fields and their visibilities from other crates if necessary for error reporting, but it should not be done on happy path (if no errors are reported).

[estebank]

@guswynn I think that the new output looks reasonable and informative.

I'll try to check what can be done in the cross-crate case, but it's better done in a separate PR.

+100

[petrochenkov]

r=me after addressing the remaining nits (#76499 (comment), #76499 (comment)) and squashing commits.

[guswynn]

@petrochenkov thank you so much for the detailed review and help on the most complicated rust-lang/rust pr I have ever done!

[petrochenkov]

Thanks!
@bors r+

[bors]

📌 Commit c63f634 has been approved by petrochenkov

[bors]

⌛ Testing commit c63f634 with merge bc57bd8...

[bors]

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing bc57bd8 to master...