When possible point at argument causing item obligation failure by estebank · Pull Request #64498 · rust-lang/rust

estebank · 2019-09-16T05:02:29Z

Fix #41781, fix #42855, fix #46658, fix #48099, fix #63143, fix #36775.

rust-highfive · 2019-09-16T05:02:33Z

r? @zackmdavis

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

estebank · 2019-09-16T05:06:15Z

Related, but not affected by this PR #61860.

This will likely conflict with #64151.

Follow up to #63870.

estebank · 2019-09-16T05:13:10Z

CC @oli-obk @Centril

I want to acknowledge that this is not the ideal solution, but my attempts at modifying the obligation preemptively and collecting an association between the FnDecl input fields and the bounds was met with some trouble because the FnDecl has hir::Ty while the trait bounds could be checked against ty::TyS. The solution in this PR is 1) partial/incomplete 2) hacky 3) quickly bails if there isn't only one argument that matches 4) somehow increasing the number of E0277 errors being emitted in some cases 5) regressing the errors in ? 6) good enough for the cases most likely to be encountered.

Centril · 2019-09-16T12:56:37Z

src/librustc_typeck/check/mod.rs

Seems like this is going to regress happy-path compile-time perf? (I don't expect by much but still...)

I see no way of doing this without collecting the resolved types for the general case (there's a lazy way to do it that would only work with fully resolved types in the arguments).

Yeah sure, just pointing out that there is a cost to this, which may be totally acceptable. :)

src/librustc_typeck/check/mod.rs

estebank · 2019-09-16T17:19:45Z

src/test/ui/issues/issue-28098.rs

This file shows the case where we duplicate the errors. We should be able to elide the error pointing at Iterator::next with some judicious use of Ty::Error.

estebank · 2019-09-16T22:55:59Z

@oli-obk d38e090#diff-4cee39a7c18e8b5ec52efe4563c547fbR7-R11 😀

error[E0277]: the trait bound `fn() -> impl std::future::Future {foo}: std::future::Future` is not satisfied
  --> $DIR/async-fn-ctor-passed-as-arg-where-it-should-have-been-called.rs:9:9
   |
LL | fn bar(f: impl Future<Output=()>) {}
   | --------------------------------- required by `bar`
...
LL |     bar(foo);
   |         ^^^
   |         |
   |         the trait `std::future::Future` is not implemented for `fn() -> impl std::future::Future {foo}`
   |         help: use parentheses to call the function: `foo()`

src/librustc/traits/mod.rs

src/librustc/traits/error_reporting.rs

src/librustc_typeck/check/mod.rs

Centril · 2019-09-19T21:32:55Z

r? @Centril

r=me with the elaboration you gave me privately on Discord added as a comment in some fashion. :)

estebank · 2019-09-19T23:58:21Z

@bors r=Centril

bors · 2019-09-19T23:58:22Z

📌 Commit c34d9e6 has been approved by Centril

When possible point at argument causing item obligation failure Fix rust-lang#41781, fix rust-lang#42855, fix rust-lang#46658, fix rust-lang#48099, fix rust-lang#63143.

@ghost

Rollup of 8 pull requests Successful merges: - #64136 (Document From trait for LhsExpr in parser) - #64342 (factor out pluralisation remains after #64280) - #64387 (Fix redundant semicolon lint interaction with proc macro attributes) - #64498 (When possible point at argument causing item obligation failure) - #64615 (rustbuild: Turn down compression on exe installers) - #64617 (rustbuild: Turn down compression on msi installers) - #64618 (rustbuild: Improve output of `dist` step) - #64621 (Add Compatibility Notes to RELEASES.md for 1.38.0) Failed merges: r? @ghost

bors · 2019-09-20T07:53:30Z

⌛ Testing commit c34d9e6 with merge 7225264...

When possible point at argument causing item obligation failure Fix #41781, fix #42855, fix #46658, fix #48099, fix #63143.

bors · 2019-09-20T11:38:51Z

☀️ Test successful - checks-azure
Approved by: Centril
Pushing 7225264 to master...

nnethercote · 2019-09-23T00:00:42Z

Either this PR or #64584 caused a major regression in rustc perf. It's hard to tell because the graph goes up on this PR, then down for a single run, then stabilizes again at the worse performance after #64584.

I suggest backing out both PRs, opening new PRs, and then doing perf runs on both. That should make it clear which one cause the regression.

cc @rust-lang/compiler

(If I had to guess, I would guess this PR caused the regression.)

Mark-Simulacrum · 2019-09-23T01:40:57Z

See #64584 (comment) - I wouldn't do anything just yet.

rust-highfive assigned zackmdavis Sep 16, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 16, 2019

This comment has been minimized.

Sign in to view

estebank force-pushed the point-at-arg branch from 8cd714c to 0551498 Compare September 16, 2019 05:13

Centril reviewed Sep 16, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

Centril reviewed Sep 16, 2019

View reviewed changes

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved

estebank force-pushed the point-at-arg branch from e717735 to e16b1bc Compare September 16, 2019 17:17

This comment has been minimized.

Sign in to view

estebank commented Sep 16, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

estebank force-pushed the point-at-arg branch from e16b1bc to 64c9bcf Compare September 16, 2019 21:42

Centril reviewed Sep 17, 2019

View reviewed changes

Centril reviewed Sep 18, 2019

View reviewed changes

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved

estebank added 7 commits September 19, 2019 12:10

When possible point at argument causing item obligation failure

02e3fb8

Ignore obligations coming from desugared call spans

fa496c9

Review comment: move to its own method

5976e0e

When possible, suggest fn call

2fbd692

review comments

c1ed439

review comments

09f05d2

remove duplicated code

d64c90d

rust-highfive assigned Centril and unassigned zackmdavis Sep 19, 2019

Centril 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 Sep 19, 2019

add comments

c34d9e6

estebank force-pushed the point-at-arg branch from 6f5a961 to c34d9e6 Compare September 19, 2019 21:52

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 19, 2019

Centril mentioned this pull request Sep 20, 2019

Rollup of 8 pull requests #64626

Closed

bors added a commit that referenced this pull request Sep 20, 2019

Auto merge of #64498 - estebank:point-at-arg, r=Centril

7225264

When possible point at argument causing item obligation failure Fix #41781, fix #42855, fix #46658, fix #48099, fix #63143.

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 20, 2019

bors merged commit c34d9e6 into rust-lang:master Sep 20, 2019

bors mentioned this pull request Sep 20, 2019

Object safe for dispatch #57545

Merged

jebrosen mentioned this pull request Sep 21, 2019

Remove crate_visibility_modifier feature rwf2/Rocket#1143

Closed

estebank mentioned this pull request Sep 22, 2019

Suboptimal error reporting of invalid argument to generic function #36775

Closed

nnethercote mentioned this pull request Sep 23, 2019

record fewer adjustment types in generator witnesses, avoid spurious drops in MIR construction #64584

Merged

estebank mentioned this pull request Jun 18, 2020

Confusing 'type mismatch' error when using function pointer as fn trait #73457

Open

Uh oh!

Conversation

estebank commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Sep 16, 2019

Uh oh!

This comment has been minimized.

estebank commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

estebank commented Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Centril Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

estebank Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

Centril Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

This comment has been minimized.

estebank Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

estebank commented Sep 16, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Centril commented Sep 19, 2019

Uh oh!

estebank commented Sep 19, 2019

Uh oh!

bors commented Sep 19, 2019

Uh oh!

bors commented Sep 20, 2019

Uh oh!

bors commented Sep 20, 2019

Uh oh!

nnethercote commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum commented Sep 23, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

estebank commented Sep 16, 2019 •

edited

Loading

estebank commented Sep 16, 2019 •

edited

Loading

estebank commented Sep 16, 2019 •

edited

Loading

nnethercote commented Sep 23, 2019 •

edited

Loading