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

Tweak borrow error on `FnMut` when `Fn` is expected #68816

Merged
merged 2 commits into from Feb 11, 2020

Conversation

@estebank
Copy link
Contributor

estebank commented Feb 4, 2020

Fix #31701, fix #66097.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Feb 4, 2020

r? @eddyb

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

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Feb 4, 2020

r? @varkor

@rust-highfive rust-highfive assigned varkor and unassigned eddyb Feb 4, 2020
@estebank estebank force-pushed the estebank:fn-mut-closure branch from 8f9dff8 to 532623b Feb 4, 2020
@rust-highfive

This comment was marked as resolved.

Copy link
Collaborator

rust-highfive commented Feb 4, 2020

The job x86_64-gnu-llvm-7 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.
2020-02-04T00:41:46.8057423Z ========================== Starting Command Output ===========================
2020-02-04T00:41:46.8059115Z [command]/bin/bash --noprofile --norc /home/vsts/work/_temp/f2b78b9e-f0d3-47a7-acb5-ec7a8cd1177e.sh
2020-02-04T00:41:46.8059153Z 
2020-02-04T00:41:46.8061992Z ##[section]Finishing: Disable git automatic line ending conversion
2020-02-04T00:41:46.8067836Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68816/merge to s
2020-02-04T00:41:46.8069965Z Task         : Get sources
2020-02-04T00:41:46.8070000Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-04T00:41:46.8070032Z Version      : 1.0.0
2020-02-04T00:41:46.8070065Z Author       : Microsoft
---
2020-02-04T00:41:47.8001834Z ##[command]git remote add origin https://github.com/rust-lang/rust
2020-02-04T00:41:47.8010369Z ##[command]git config gc.auto 0
2020-02-04T00:41:47.8012190Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2020-02-04T00:41:47.8014139Z ##[command]git config --get-all http.proxy
2020-02-04T00:41:47.8021099Z ##[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/68816/merge:refs/remotes/pull/68816/merge
---
2020-02-04T00:47:03.9491332Z Found 0 error codes with no tests
2020-02-04T00:47:03.9491464Z Done!
2020-02-04T00:47:03.9494173Z 
2020-02-04T00:47:03.9494357Z 
2020-02-04T00:47:03.9495329Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/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"
2020-02-04T00:47:03.9495679Z 
2020-02-04T00:47:03.9495780Z 
2020-02-04T00:47:03.9503441Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
2020-02-04T00:47:03.9504777Z Build completed unsuccessfully in 0:01:24
2020-02-04T00:47:03.9504777Z Build completed unsuccessfully in 0:01:24
2020-02-04T00:47:03.9554095Z == clock drift check ==
2020-02-04T00:47:03.9562058Z   local time: Tue Feb  4 00:47:03 UTC 2020
2020-02-04T00:47:04.2468356Z   network time: Tue, 04 Feb 2020 00:47:04 GMT
2020-02-04T00:47:04.2473120Z == end clock drift check ==
2020-02-04T00:47:04.9934309Z 
2020-02-04T00:47:05.0014713Z ##[error]Bash exited with code '1'.
2020-02-04T00:47:05.0025045Z ##[section]Finishing: Run build
2020-02-04T00:47:05.0037940Z ##[section]Starting: Checkout rust-lang/rust@refs/pull/68816/merge to s
2020-02-04T00:47:05.0039455Z Task         : Get sources
2020-02-04T00:47:05.0039492Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.
2020-02-04T00:47:05.0039546Z Version      : 1.0.0
2020-02-04T00:47:05.0039578Z Author       : Microsoft
2020-02-04T00:47:05.0039578Z Author       : Microsoft
2020-02-04T00:47:05.0039612Z Help         : [More Information](https://go.microsoft.com/fwlink/?LinkId=798199)
2020-02-04T00:47:05.0039682Z ==============================================================================
2020-02-04T00:47:05.3674610Z Cleaning any cached credential from repository: rust-lang/rust (GitHub)
2020-02-04T00:47:05.3716863Z ##[section]Finishing: Checkout rust-lang/rust@refs/pull/68816/merge to s
2020-02-04T00:47:05.3941642Z Cleaning up task key
2020-02-04T00:47:05.3943579Z Start cleaning up orphan processes.
2020-02-04T00:47:05.4085095Z Terminate orphan process: pid (3667) (python)
2020-02-04T00:47:05.4297677Z ##[section]Finishing: Finalize Job

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 estebank force-pushed the estebank:fn-mut-closure branch from 532623b to 109d5c1 Feb 4, 2020
Copy link
Member

varkor left a comment

The changes look good! Just one comment.

LL | fn foo() -> Box<dyn Fn() -> usize> {
| --- ---------------------- ...to return `FnMut` instead of `Fn`
| |
| you might have to change this...

This comment has been minimized.

Copy link
@varkor

varkor Feb 9, 2020

Member

The order of this message seems reversed with the message above.

This comment has been minimized.

Copy link
@estebank

estebank Feb 9, 2020

Author Contributor

What about in this function.../...you might have to change this to return FnMut instead?

This comment has been minimized.

Copy link
@varkor

varkor Feb 9, 2020

Member

The actual text is fine — I'm just inclined to read the topmost line first:

...to return `FnMut` instead of `Fn`
you might have to change this...

when it's intended to be read in the opposite order.

This comment has been minimized.

Copy link
@estebank

estebank Feb 9, 2020

Author Contributor

I kept the span for the method ident with no label and shortened the wording to change this to return FnMut instead of Fn, as it seems more consistent with the terseness of the other suggestions we give.

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Feb 10, 2020

@bors r=varkor based on #68816 (review)

@estebank

This comment has been minimized.

Copy link
Contributor Author

estebank commented Feb 10, 2020

@bors r=varkor

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 10, 2020

📌 Commit d51f2bd has been approved by varkor

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 10, 2020

🌲 The tree is currently closed for pull requests below priority 10, this pull request will be tested once the tree is reopened

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 11, 2020
Tweak borrow error on `FnMut` when `Fn` is expected

Fix rust-lang#31701, fix rust-lang#66097.
bors added a commit that referenced this pull request Feb 11, 2020
Rollup of 8 pull requests

Successful merges:

 - #66498 (Remove unused feature gates)
 - #68816 (Tweak borrow error on `FnMut` when `Fn` is expected)
 - #68824 (Enable Control Flow Guard in rustbuild)
 - #69022 (traits: preallocate 2 Vecs of known initial size)
 - #69031 (Use `dyn Trait` more in tests)
 - #69044 (Don't run coherence twice for future-compat lints)
 - #69047 (Don't rustfmt check the vendor directory.)
 - #69055 (Clean up E0307 explanation)

Failed merges:

r? @ghost
@bors bors merged commit d51f2bd into rust-lang:master Feb 11, 2020
4 checks passed
4 checks passed
pr Build #20200209.13 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.