Skip to content

Make TyS::is_suggestable check for non-suggestable types structually #91898

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged

Conversation

compiler-errors
Copy link
Member

Not sure if I went overboard checking substs in dyn types, etc. Let me know if I should simplify this function.

Fixes #91832

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 14, 2021
@rust-highfive
Copy link
Contributor

r? @cjgillot

(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 Dec 14, 2021
@compiler-errors
Copy link
Member Author

cc: @estebank, who might care about changes in diagnostic type annotations.

fn generic_arg_is_suggestible(arg: GenericArg<'_>) -> bool {
match arg.unpack() {
GenericArgKind::Type(ty) => ty.is_suggestable(),
_ => true,
Copy link
Contributor

Choose a reason for hiding this comment

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

it probably makes sense to also return false for ConstKind::Infer | ConstKind::Bound | ConstKind::Placeholder | ConstKind::Error here. And also look at array lengths now.

After that r=me unless @estebank or @cjgillot want some other changes here

@compiler-errors compiler-errors force-pushed the dont_suggest_closure_return_type branch from c457ee2 to 2215186 Compare December 14, 2021 19:30
@compiler-errors compiler-errors force-pushed the dont_suggest_closure_return_type branch from 2215186 to f29fb47 Compare December 14, 2021 19:32
@lcnr
Copy link
Contributor

lcnr commented Dec 15, 2021

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 15, 2021

📌 Commit f29fb47 has been approved by lcnr

@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 15, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Dec 15, 2021
…askrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#91880 (fix clippy::single_char_pattern perf findings)
 - rust-lang#91885 (Remove `in_band_lifetimes` from `rustc_codegen_ssa`)
 - rust-lang#91898 (Make `TyS::is_suggestable` check for non-suggestable types structually)
 - rust-lang#91915 (Add another regression test for unnormalized fn args with Self)
 - rust-lang#91916 (Fix a bunch of typos)
 - rust-lang#91918 (Constify `bool::then{,_some}`)
 - rust-lang#91920 (Use `tcx.def_path_hash` in `ExistentialPredicate.stable_cmp`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b507174 into rust-lang:master Dec 15, 2021
@rustbot rustbot added this to the 1.59.0 milestone Dec 15, 2021
@estebank
Copy link
Contributor

I was sad to see some of the suggestions go away, so I created #91981 that builds on top of this one.

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 27, 2022
Recover suggestions and useful information lost in previous PR

Follow up to rust-lang#91898.
@compiler-errors compiler-errors deleted the dont_suggest_closure_return_type branch April 7, 2022 04:36
# for free to join this conversation on GitHub. Already have an account? # 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.

error help suggests writing down a closure type, which would be a syntax error
7 participants