ty: return impl Iterator from Predicate::walk_tys by ljedrz · Pull Request #55949 · rust-lang/rust

ljedrz · 2018-11-14T14:05:34Z

Fixes the lazyboye FIXME by returning a custom Iterator as intended by the original author of the function.

It is indeed a bit convoluted, so I'm ok with not changing this if perf results are not favourable enough. Also happy to adjust any names if need be.

rust-highfive · 2018-11-14T14:05:37Z

r? @petrochenkov

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

wesleywiser

small nit otherwise lgtm

wesleywiser · 2018-11-14T16:05:27Z

@bors try

Let's get the perf run started.

bors · 2018-11-14T16:05:42Z

⌛ Trying commit daaad757802bcc25d6f273f0c80305955ff68c0f with merge 2721a905baf123efea5ca0838895ecd9c8ad142b...

bors · 2018-11-14T18:38:59Z

☀️ Test successful - status-travis
State: approved= try=True

wesleywiser · 2018-11-14T18:41:26Z

@rust-timer build 2721a905baf123efea5ca0838895ecd9c8ad142b

rust-timer · 2018-11-14T18:41:27Z

Insufficient permissions to issue commands to rust-timer.

ljedrz · 2018-11-14T19:32:06Z

@kennytm can you help with rust-timer please?

alexcrichton · 2018-11-14T23:22:15Z

@rust-timer build 2721a905baf123efea5ca0838895ecd9c8ad142b

rust-timer · 2018-11-14T23:22:16Z

Success: Queued 2721a905baf123efea5ca0838895ecd9c8ad142b with parent 6f244c9, comparison URL.

rust-timer · 2018-11-15T02:16:34Z

Finished benchmarking try commit 2721a905baf123efea5ca0838895ecd9c8ad142b

ljedrz · 2018-11-15T08:36:35Z

The perf looks pretty innocuous, but nothing too exciting; perhaps the ctfe and stress benchmarks have improved a little bit, as they have pretty consistent instruction count deltas.

sinkuu · 2018-11-15T10:45:08Z

This method is only used by rustdoc, so the perf changes may be noise.

> rg -w walk_tys src
src/librustc/ty/mod.rs
1350: pub fn walk_tys(&self) -> IntoIter<Ty<'tcx>> {
src/librustc/ty/sty.rs
1855: /// types reachable from this type via `walk_tys`). This ignores late-bound
src/librustdoc/clean/auto_trait.rs
408: pred.walk_tys()

ljedrz · 2018-11-15T11:17:54Z

Oh; it is entirely possible then 😆.

oli-obk · 2018-11-15T11:40:25Z

@bors r+ rollup

bors · 2018-11-15T11:40:26Z

📌 Commit e6e5635 has been approved by oli-obk

…edicate_walk_tys, r=oli-obk ty: return impl Iterator from Predicate::walk_tys Fixes the lazyboye `FIXME` by returning a custom `Iterator` as intended by the original author of the function. It is indeed a bit convoluted, so I'm ok with not changing this if perf results are not favourable enough. Also happy to adjust any names if need be.

@ghost

Rollup of 25 pull requests Successful merges: - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets) - #55564 (test/linkage-visibility: Ignore on musl targets) - #55827 (A few tweaks to iterations/collecting) - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI) - #55857 (remove unused dependency) - #55862 (in which the E0618 "expected function" diagnostic gets a makeover) - #55867 (do not panic just because cargo failed) - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef) - #55916 (Make miri value visitor usfeful for mutation) - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code) - #55923 (reword #[test] attribute error on fn items) - #55935 (appveyor: Use VS2017 for all our images) - #55949 (ty: return impl Iterator from Predicate::walk_tys) - #55952 (Update to Clang 7 on CI.) - #55953 (#53488 Refactoring UpvarId) - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors) - #55963 (Stress test for MPSC) - #55968 (Clean up some non-mod-rs stuff.) - #55970 (Miri backtrace improvements) - #56007 (CTFE: dynamically make sure we do not call non-const-fn) - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.) - #56012 (avoid shared ref in UnsafeCell::get) - #56016 (Add VecDeque::resize_with) - #56027 (docs: Add missing backtick in object_safety.rs docs) - #56043 (remove "approx env bounds" if we already know from trait) Failed merges: r? @ghost

…edicate_walk_tys, r=oli-obk ty: return impl Iterator from Predicate::walk_tys Fixes the lazyboye `FIXME` by returning a custom `Iterator` as intended by the original author of the function. It is indeed a bit convoluted, so I'm ok with not changing this if perf results are not favourable enough. Also happy to adjust any names if need be.

Rollup of 25 pull requests Successful merges: - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets) - #55564 (test/linkage-visibility: Ignore on musl targets) - #55827 (A few tweaks to iterations/collecting) - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI) - #55857 (remove unused dependency) - #55862 (in which the E0618 "expected function" diagnostic gets a makeover) - #55867 (do not panic just because cargo failed) - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef) - #55916 (Make miri value visitor useful for mutation) - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code) - #55923 (reword #[test] attribute error on fn items) - #55949 (ty: return impl Iterator from Predicate::walk_tys) - #55952 (Update to Clang 7 on CI.) - #55953 (#53488 Refactoring UpvarId) - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors) - #55963 (Stress test for MPSC) - #55968 (Clean up some non-mod-rs stuff.) - #55970 (Miri backtrace improvements) - #56007 (CTFE: dynamically make sure we do not call non-const-fn) - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.) - #56012 (avoid shared ref in UnsafeCell::get) - #56016 (Add VecDeque::resize_with) - #56027 (docs: Add missing backtick in object_safety.rs docs) - #56043 (remove "approx env bounds" if we already know from trait) - #56059 (Increase `Duration` approximate equal threshold to 1us)

rust-highfive assigned petrochenkov Nov 14, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 14, 2018

wesleywiser reviewed Nov 14, 2018
View reviewed changes

Comment threadsrc/librustc/ty/mod.rs Outdated

ljedrz force-pushed the return_impl_Iterator_from_Predicate_walk_tys branch from faf78e0 to daaad75Compare November 14, 2018 15:36

wesleywiser approved these changes Nov 14, 2018
View reviewed changes

Comment threadsrc/librustc/ty/mod.rs Outdated

oli-obk reviewed Nov 15, 2018
View reviewed changes

Comment threadsrc/librustc/ty/mod.rs Outdated

ty: return impl Iterator from Predicate::walk_tys
e6e5635

ljedrz force-pushed the return_impl_Iterator_from_Predicate_walk_tys branch from daaad75 to e6e5635Compare November 15, 2018 08:27

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 Nov 15, 2018

emilyalbini mentioned this pull request Nov 15, 2018
Rollup of 15 pull requests #55990
Closed

petrochenkov assigned oli-obk and unassigned petrochenkov Nov 16, 2018

kennytm mentioned this pull request Nov 17, 2018
Rollup of 17 pull requests #56026
Closed

emilyalbini mentioned this pull request Nov 18, 2018
Rollup of 25 pull requests #56051
Merged

bors merged commit e6e5635 into rust-lang:masterNov 19, 2018

ljedrz deleted the return_impl_Iterator_from_Predicate_walk_tys branch November 19, 2018 17:10

Uh oh!

Uh oh!

Conversation

ljedrz commented Nov 14, 2018

Uh oh!

rust-highfive commented Nov 14, 2018

Uh oh!

Uh oh!

wesleywiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wesleywiser commented Nov 14, 2018

Uh oh!

bors commented Nov 14, 2018

Uh oh!

bors commented Nov 14, 2018

Uh oh!

wesleywiser commented Nov 14, 2018

Uh oh!

rust-timer commented Nov 14, 2018

Uh oh!

ljedrz commented Nov 14, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexcrichton commented Nov 14, 2018

Uh oh!

rust-timer commented Nov 14, 2018

Uh oh!

rust-timer commented Nov 15, 2018

Uh oh!

Uh oh!

ljedrz commented Nov 15, 2018

Uh oh!

sinkuu commented Nov 15, 2018

Uh oh!

ljedrz commented Nov 15, 2018

Uh oh!

oli-obk commented Nov 15, 2018

Uh oh!

bors commented Nov 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

ljedrz commented Nov 14, 2018•
edited
Loading