Skip to content

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Nov 8, 2021

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.

@rust-highfive
Copy link
Contributor

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 8, 2021
@apiraino apiraino added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Nov 11, 2021
Comment on lines 953 to 954
// Replace not yet inferred const params with their def name.
self.tcx().mk_const_param(0, Symbol::intern("N"), c.ty).into()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment correct?

Copy link
Member

@nagisa nagisa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test where multiple parameters are not inferred would be nice too. Of particular interest would be a function that returns an array where both the value type and const length could not be inferred.

ty::Infer(_) => t,
// We don't want to hide the outermost type, only its type params.
_ if self.level == 1 => t.super_fold_with(self),
// Hide this type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm slightly worried about the handling of ty::Errors that come in as part of the input as they become indistinguishable from those that are produced by this TypeFolder, but its probably fine…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would think that any naturally ocurring ty::Error should be displayed as _ anyways. Worst case scenario on the next wave of errors the user will be told to fill it in.

@nagisa nagisa 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 Nov 19, 2021
@estebank estebank force-pushed the erase-known-type-params branch 2 times, most recently from 5c807b1 to 0f668ff Compare November 19, 2021 05:56
@estebank 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 Nov 19, 2021
@rust-log-analyzer

This comment has been minimized.

Comment on lines +15 to +18
help: consider specifying the type arguments in the function call
|
LL | let foo = foo::<T, K, W, Z>(1, "");
| ++++++++++++++
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outstanding: changing this to also use _ for the first two params here. Can be done as a follow up.

@bors
Copy link
Collaborator

bors commented Nov 22, 2021

☔ The latest upstream changes (presumably #90352) made this pull request unmergeable. Please resolve the merge conflicts.

@estebank estebank force-pushed the erase-known-type-params branch from 3ce5b5e to e462819 Compare November 22, 2021 18:58
@nagisa
Copy link
Member

nagisa commented Nov 27, 2021

@bors r+

@estebank
Copy link
Contributor Author

@bors r=nagisa

The bot seems to have been taking a nap.

@bors
Copy link
Collaborator

bors commented Nov 29, 2021

📌 Commit e462819 has been approved by nagisa

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Nov 29, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Nov 30, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 1, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
@matthiaskrgr
Copy link
Member

Failed in a rollup, maybe just needs a small rebase and fixup..?
#91413 (comment)
@bors r- rollup=iffy

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 1, 2021
@estebank estebank force-pushed the erase-known-type-params branch from e462819 to b988151 Compare December 2, 2021 21:04
@estebank
Copy link
Contributor Author

estebank commented Dec 2, 2021

@bors r=nagisa

@bors
Copy link
Collaborator

bors commented Dec 3, 2021

⌛ Testing commit b988151 with merge 21f0c7aafcdf0c6622a2f03305a57efcfb2cbf79...

@bors
Copy link
Collaborator

bors commented Dec 3, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 3, 2021
@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Collaborator

bors commented Dec 5, 2021

☔ The latest upstream changes (presumably #91539) made this pull request unmergeable. Please resolve the merge conflicts.

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
When the value of a const param isn't inferred, replace it with the
param name from the definition.
@estebank estebank force-pushed the erase-known-type-params branch from b988151 to 7271d1f Compare December 7, 2021 02:19
@estebank
Copy link
Contributor Author

estebank commented Dec 7, 2021

@bors r=nagisa

@bors
Copy link
Collaborator

bors commented Dec 7, 2021

📌 Commit 7271d1f has been approved by nagisa

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 7, 2021
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 8, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#90709 (Only shown relevant type params in E0283 label)
 - rust-lang#91551 (Allow for failure of subst_normalize_erasing_regions in const_eval)
 - rust-lang#91570 (Evaluate inline const pat early and report error if too generic)
 - rust-lang#91571 (Remove unneeded access to pretty printer's `s` field in favor of deref)
 - rust-lang#91610 (Link to rustdoc_json_types docs instead of rustdoc-json RFC)
 - rust-lang#91619 (Update cargo)
 - rust-lang#91630 (Add missing whitespace before disabled HTML attribute)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 8, 2021
…=nagisa

Only shown relevant type params in E0283 label

When we point at a binding to suggest giving it a type, erase all the
type for ADTs that have been resolved, leaving only the ones that could
not be inferred. For small shallow types this is not a problem, but for
big nested types with lots of params, this can otherwise cause a lot of
unnecessary visual output.
@bors bors merged commit 7970fab into rust-lang:master Dec 8, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants