Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Account for bad placeholder types in where clauses #70294

Merged
merged 1 commit into from
Mar 24, 2020

Conversation

estebank
Copy link
Contributor

@estebank estebank commented Mar 23, 2020

Fix #70291. Follow up to #69148.

@rust-highfive
Copy link
Collaborator

r? @ecstatic-morse

(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 Mar 23, 2020
@estebank estebank added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 23, 2020
src/librustc_typeck/collect.rs Outdated Show resolved Hide resolved
src/librustc_typeck/astconv.rs Outdated Show resolved Hide resolved
@estebank
Copy link
Contributor Author

r? @Centril

@estebank estebank force-pushed the bad-placeholder-in-where branch from 0410eee to 5af9042 Compare March 23, 2020 03:29
@estebank estebank force-pushed the bad-placeholder-in-where branch from 5af9042 to 7c8fe98 Compare March 23, 2020 06:18
@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Contributor

bors commented Mar 23, 2020

⌛ Trying commit 7c8fe98681ac93740c1c959f0b4951ad090034ee with merge 50962f2a2876523fc8f966266bee06123a5dcd3b...

@Centril Centril added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 23, 2020
@bors
Copy link
Contributor

bors commented Mar 23, 2020

☀️ Try build successful - checks-azure
Build commit: 50962f2a2876523fc8f966266bee06123a5dcd3b (50962f2a2876523fc8f966266bee06123a5dcd3b)

@rust-timer
Copy link
Collaborator

Queued 50962f2a2876523fc8f966266bee06123a5dcd3b with parent 37c945d, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 50962f2a2876523fc8f966266bee06123a5dcd3b, comparison URL.

@estebank
Copy link
Contributor Author

This as it stands doesn't handle fn foo<F: Fn() -> _>(_: F) {}. I'll fix that.

@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

@estebank does it ICE in that case? If not, then let's not fix it in this PR.

@estebank
Copy link
Contributor Author

estebank commented Mar 23, 2020

It does ICE. Changing this to use walk_generics(&mut visitor, generics); makes the ICE go away at the cost of a duplicated error. I'll see if I can get rid of the dupe, but if not I prefer that over an ICE.

@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

@estebank Could we do the simple thing in this PR for backportability reasons and follow up with possible de-dup improvements?

@estebank estebank force-pushed the bad-placeholder-in-where branch from 7c8fe98 to b8bca24 Compare March 23, 2020 19:35
@estebank
Copy link
Contributor Author

I'm comfortable with the current state of the PR.

Comment on lines -841 to -852
if !inferred_params.is_empty() {
// We always collect the spans for placeholder types when evaluating `fn`s, but we
// only want to emit an error complaining about them if infer types (`_`) are not
// allowed. `allow_ty_infer` gates this behavior.
crate::collect::placeholder_type_error(
tcx,
inferred_params[0],
&[],
inferred_params,
false,
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this got rid of the duplicated errors.

@Centril
Copy link
Contributor

Centril commented Mar 23, 2020

I am too, let's :shipit:

@bors r+

@bors
Copy link
Contributor

bors commented Mar 23, 2020

📌 Commit b8bca241cb7f372170bdc628ef2cdbc19f7d3f14 has been approved by Centril

@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 Mar 23, 2020
@estebank estebank force-pushed the bad-placeholder-in-where branch from b8bca24 to e75158d Compare March 23, 2020 19:55
@estebank
Copy link
Contributor Author

rustfmt an import 🤦‍♂️

@bors r=Centril

@bors
Copy link
Contributor

bors commented Mar 23, 2020

📌 Commit e75158d has been approved by Centril

Centril added a commit to Centril/rust that referenced this pull request Mar 23, 2020
…r=Centril

Account for bad placeholder types in where clauses

Fix rust-lang#70291. Follow up to rust-lang#69148.
Centril added a commit to Centril/rust that referenced this pull request Mar 23, 2020
…r=Centril

Account for bad placeholder types in where clauses

Fix rust-lang#70291. Follow up to rust-lang#69148.
This was referenced Mar 23, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 24, 2020
Rollup of 11 pull requests

Successful merges:

 - rust-lang#67761 (Move the dep_graph construction to a dedicated crate.)
 - rust-lang#69740 (Replace some desc logic in librustc_lint with article_and_desc)
 - rust-lang#69981 (Evaluate repeat expression lengths as late as possible)
 - rust-lang#70087 (Remove const eval loop detector)
 - rust-lang#70242 (Improve E0308 error message wording)
 - rust-lang#70264 (Fix invalid suggestion on `&mut` iterators yielding `&` references)
 - rust-lang#70267 (get rid of ConstPropUnsupported; use ZST marker structs instead)
 - rust-lang#70277 (Remove `ReClosureBound`)
 - rust-lang#70283 (Add regression test for rust-lang#70155.)
 - rust-lang#70294 (Account for bad placeholder types in where clauses)
 - rust-lang#70309 (Clean up E0452 explanation)

Failed merges:

r? @ghost
@bors bors merged commit 54db0cf into rust-lang:master Mar 24, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 25, 2020
…r=Centril

Fix smaller issues with invalid placeholder type errors

Follow up to rust-lang#70294.

- Fix placement of suggested generic param when bounds are present.
- Reduce error duplication for invalid placeholder types in `fn` types.

r? @Centril
Centril added a commit to Centril/rust that referenced this pull request Mar 25, 2020
…r=Centril

Fix smaller issues with invalid placeholder type errors

Follow up to rust-lang#70294.

- Fix placement of suggested generic param when bounds are present.
- Reduce error duplication for invalid placeholder types in `fn` types.

r? @Centril
@pnkfelix
Copy link
Member

pnkfelix commented Apr 1, 2020

Discussed in T-compiler meeting. Accepted for beta-backport.

@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Apr 1, 2020
@Mark-Simulacrum Mark-Simulacrum removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Apr 3, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 3, 2020
…ulacrum

[beta] backport 4 PRs

This backports the following PRs:

* parse_and_disallow_postfix_after_cast: account for `ExprKind::Err`. rust-lang#70556
* Account for bad placeholder types in where clauses rust-lang#70294
* Fix "since" field for `Once::is_complete`'s `#[stable]` attribute rust-lang#70018
* Ensure HAS_FREE_LOCAL_NAMES is set for ReFree rust-lang#69956

All commits cherry picked cleanly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.

"bad placeholder type" ICE regression
8 participants