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

Use structured suggestion for restricting bounds #65192

Merged
merged 16 commits into from Oct 19, 2019

Conversation

@estebank
Copy link
Contributor

estebank commented Oct 7, 2019

When a trait bound is not met and restricting a type parameter would
make the restriction hold, use a structured suggestion pointing at an
appropriate place (type param in param list or where clause).

Account for opaque parameters where instead of suggesting extending
the where clause, we suggest appending the new restriction:
fn foo(impl Trait + UnmetTrait). Fix #64565, fix #41817, fix #24354,
cc #26026, cc #37808, cc #24159, fix #37138, fix #24354, cc #20671.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Oct 7, 2019

r? @matthewjasper

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

@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Oct 7, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-07T21:27:26.4275315Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-07T21:27:26.4451790Z ##[command]git config gc.auto 0
2019-10-07T21:27:26.4519643Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-07T21:27:26.4574367Z ##[command]git config --get-all http.proxy
2019-10-07T21:27:26.4709765Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65192/merge:refs/remotes/pull/65192/merge
---
2019-10-07T21:34:02.8145210Z    Compiling serde_json v1.0.40
2019-10-07T21:34:04.6201092Z    Compiling tidy v0.1.0 (/checkout/src/tools/tidy)
2019-10-07T21:34:15.5858243Z     Finished release [optimized] target(s) in 1m 28s
2019-10-07T21:34:15.5929491Z tidy check
2019-10-07T21:34:16.2143380Z tidy error: /checkout/src/librustc/traits/error_reporting.rs:721: line longer than 100 chars
2019-10-07T21:34:17.4830599Z some tidy checks failed
2019-10-07T21:34:17.4836659Z 
2019-10-07T21:34:17.4836659Z 
2019-10-07T21:34:17.4837601Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor"
2019-10-07T21:34:17.4837781Z 
2019-10-07T21:34:17.4837806Z 
2019-10-07T21:34:17.4845118Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2019-10-07T21:34:17.4845195Z Build completed unsuccessfully in 0:01:31
2019-10-07T21:34:17.4845195Z Build completed unsuccessfully in 0:01:31
2019-10-07T21:34:17.4897140Z == clock drift check ==
2019-10-07T21:34:17.4912234Z   local time: Mon Oct  7 21:34:17 UTC 2019
2019-10-07T21:34:17.5889463Z   network time: Mon, 07 Oct 2019 21:34:17 GMT
2019-10-07T21:34:17.5893374Z == end clock drift check ==
2019-10-07T21:34:18.9577456Z ##[error]Bash exited with code '1'.
2019-10-07T21:34:18.9641677Z ##[section]Starting: Checkout
2019-10-07T21:34:18.9643783Z ==============================================================================
2019-10-07T21:34:18.9643851Z Task         : Get sources
2019-10-07T21:34:18.9643904Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

let restrict_msg = "consider further restricting this bound";
let param_name = param_ty.unwrap().name.as_str();
for param in &generics.params {
if param_name == param.name.ident().as_str() {

This comment has been minimized.

Copy link
@Centril

Centril Oct 10, 2019

Member

As I noted on Discord, I think this method is way too complicated not be be divided along the important match arms (or the places that involve early returns) (and it doesn't fit on a single page...). However, if you insist on keeping it all in one method then let's at least use a .filter(...) here to reduce the drift rightwards.

@estebank estebank force-pushed the estebank:restrict-bound branch 3 times, most recently from 9dd6fa6 to 7409942 Oct 10, 2019
@estebank

This comment was marked as outdated.

Copy link
Contributor Author

estebank commented Oct 10, 2019

@Centril do you think there's any reasonable general suggestion we can give for the cases changed by f9742e2? For the == case I think we should have a binop specific check somewhere else, so noone should miss the help. For the later case, I'm not sure what the message should/could be.

@Centril

This comment was marked as outdated.

Copy link
Member

Centril commented Oct 11, 2019

No idea, sorry :)

@estebank

This comment was marked as outdated.

Copy link
Contributor Author

estebank commented Oct 11, 2019

I think it's fine to just remove it.

src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/test/ui/issues/issue-38821.stderr Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
src/librustc/traits/error_reporting.rs Outdated Show resolved Hide resolved
@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Oct 15, 2019

The job mingw-check of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-10-15T20:38:33.7795619Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-10-15T20:38:33.8004478Z ##[command]git config gc.auto 0
2019-10-15T20:38:33.8084326Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-10-15T20:38:33.8153198Z ##[command]git config --get-all http.proxy
2019-10-15T20:38:34.3484411Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/65192/merge:refs/remotes/pull/65192/merge
---
2019-10-15T20:47:21.9181794Z     Checking syntax_ext v0.0.0 (/checkout/src/libsyntax_ext)
2019-10-15T20:47:40.2920840Z error[E0277]: can't compare `&std::string::String` with `syntax::symbol::LocalInternedString`
2019-10-15T20:47:40.2921421Z     --> src/librustc/traits/error_reporting.rs:1063:37
2019-10-15T20:47:40.2921744Z      |
2019-10-15T20:47:40.2922058Z 1063 |                         &param_name == p.name.ident().as_str()
2019-10-15T20:47:40.2922854Z      |                                     ^^ no implementation for `&std::string::String == syntax::symbol::LocalInternedString`
2019-10-15T20:47:40.2923539Z      = help: the trait `std::cmp::PartialEq<syntax::symbol::LocalInternedString>` is not implemented for `&std::string::String`
2019-10-15T20:47:40.2923613Z 
2019-10-15T20:47:48.5058751Z error: aborting due to previous error
2019-10-15T20:47:48.5058998Z 
---
2019-10-15T20:47:48.6911785Z == clock drift check ==
2019-10-15T20:47:48.6929415Z   local time: Tue Oct 15 20:47:48 UTC 2019
2019-10-15T20:47:48.8424059Z   network time: Tue, 15 Oct 2019 20:47:48 GMT
2019-10-15T20:47:48.8429648Z == end clock drift check ==
2019-10-15T20:47:49.6183061Z ##[error]Bash exited with code '1'.
2019-10-15T20:47:49.6237584Z ##[section]Starting: Checkout
2019-10-15T20:47:49.6239475Z ==============================================================================
2019-10-15T20:47:49.6239534Z Task         : Get sources
2019-10-15T20:47:49.6239583Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

estebank added 7 commits Oct 7, 2019
When a trait bound is not met and restricting a type parameter would
make the restriction hold, use a structured suggestion pointing at an
appropriate place (type param in param list or `where` clause).

Account for opaque parameters where instead of suggesting extending
the `where` clause, we suggest appending the new restriction:
`fn foo(impl Trait + UnmetTrait)`.
@estebank estebank force-pushed the estebank:restrict-bound branch from 9cd656c to c6dce78 Oct 15, 2019
@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Oct 18, 2019

@matthewjasper all your comments have been addressed.

@matthewjasper

This comment has been minimized.

Copy link
Contributor

matthewjasper commented Oct 19, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 19, 2019

📌 Commit c6dce78 has been approved by matthewjasper

Centril added a commit to Centril/rust that referenced this pull request Oct 19, 2019
…asper

Use structured suggestion for restricting bounds

When a trait bound is not met and restricting a type parameter would
make the restriction hold, use a structured suggestion pointing at an
appropriate place (type param in param list or `where` clause).

Account for opaque parameters where instead of suggesting extending
the `where` clause, we suggest appending the new restriction:
`fn foo(impl Trait + UnmetTrait)`. Fix rust-lang#64565, fix rust-lang#41817, fix rust-lang#24354,
cc rust-lang#26026, cc rust-lang#37808, cc rust-lang#24159, fix rust-lang#37138, fix rust-lang#24354, cc rust-lang#20671.
Centril added a commit to Centril/rust that referenced this pull request Oct 19, 2019
…asper

Use structured suggestion for restricting bounds

When a trait bound is not met and restricting a type parameter would
make the restriction hold, use a structured suggestion pointing at an
appropriate place (type param in param list or `where` clause).

Account for opaque parameters where instead of suggesting extending
the `where` clause, we suggest appending the new restriction:
`fn foo(impl Trait + UnmetTrait)`. Fix rust-lang#64565, fix rust-lang#41817, fix rust-lang#24354,
cc rust-lang#26026, cc rust-lang#37808, cc rust-lang#24159, fix rust-lang#37138, fix rust-lang#24354, cc rust-lang#20671.
bors added a commit that referenced this pull request Oct 19, 2019
Rollup of 5 pull requests

Successful merges:

 - #64007 (Add check for overlapping ranges to unreachable patterns lint)
 - #65192 (Use structured suggestion for restricting bounds)
 - #65226 (BTreeSet symmetric_difference & union optimized)
 - #65448 (rustc_codegen_ssa: remove some unnecessary Box special-casing.)
 - #65505 (Rc: value -> allocation)

Failed merges:

r? @ghost
@bors bors merged commit c6dce78 into rust-lang:master Oct 19, 2019
4 checks passed
4 checks passed
pr Build #20191015.65 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.