Separate the receiver from arguments in HIR #101261

TaKO8Ki · 2022-09-01T03:51:24Z

Related to #100232

cc @cjgillot

rustbot · 2022-09-01T03:51:28Z

Some changes occurred in need_type_info.rs

cc @lcnr

rust-highfive · 2022-09-01T03:51:29Z

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

rustbot · 2022-09-01T10:55:16Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

TaKO8Ki · 2022-09-01T10:55:18Z

@bors try

bors · 2022-09-01T10:55:28Z

⌛ Trying commit e39e6badb9f814749efd5e90b3879934f65f701d with merge 49b7bc501896135473621bcd0af177b4fbab1a5e...

compiler/rustc_hir/src/hir.rs

bors · 2022-09-01T12:45:50Z

☀️ Try build successful - checks-actions
Build commit: 49b7bc501896135473621bcd0af177b4fbab1a5e (49b7bc501896135473621bcd0af177b4fbab1a5e)

compiler-errors · 2022-09-01T15:36:23Z

cc #100427 i think

compiler/rustc_lint/src/array_into_iter.rs

compiler/rustc_lint/src/methods.rs

compiler/rustc_mir_build/src/thir/cx/expr.rs

cjgillot · 2022-09-01T17:00:13Z

compiler/rustc_passes/src/liveness.rs

+                std::iter::once(receiver)
+                    .chain(args.iter())
+                    .rev()
+                    .fold(succ, |succ, expr| self.propagate_through_expr(expr, succ))


Could you inline the first iteration of the fold?

compiler/rustc_typeck/src/check/demand.rs

src/tools/clippy/clippy_lints/src/infinite_iter.rs

src/tools/clippy/clippy_lints/src/len_zero.rs

src/tools/clippy/clippy_lints/src/methods/clone_on_copy.rs

src/tools/clippy/clippy_lints/src/methods/clone_on_ref_ptr.rs

src/tools/clippy/clippy_lints/src/minmax.rs

compiler/rustc_lint/src/methods.rs

src/tools/clippy/clippy_lints/src/methods/mod.rs

src/tools/clippy/clippy_lints/src/methods/bytecount.rs

src/tools/clippy/clippy_lints/src/methods/mod.rs

src/tools/clippy/clippy_lints/src/minmax.rs

camsteffen · 2022-09-01T18:43:21Z

src/tools/clippy/clippy_lints/src/unit_types/let_unit_value.rs

+            std::iter::once(receiver).chain(args.iter()).collect()
+        } else {
+            args.iter().collect()
+        };


let args = receiver.into_iter().chain(args);

src/tools/clippy/clippy_lints/src/unused_peekable.rs

bors · 2022-09-01T19:57:33Z

☔ The latest upstream changes (presumably #101239) made this pull request unmergeable. Please resolve the merge conflicts.

TaKO8Ki · 2022-09-02T14:44:27Z

@rustbot ready

cjgillot

This is not the simplification that we hoped for, but this makes the semantic much clearer, so let's go.
r=me with a few extra nits.

compiler/rustc_borrowck/src/diagnostics/region_errors.rs

compiler/rustc_passes/src/liveness.rs

src/tools/clippy/clippy_lints/src/eta_reduction.rs

lcnr · 2022-09-05T05:11:01Z

@bors try @rust-timer queue

bors · 2022-09-05T08:42:41Z

☀️ Try build successful - checks-actions
Build commit: 25a8c2d85ec7bbe4e5a7071f9725b3b80ec9add3 (25a8c2d85ec7bbe4e5a7071f9725b3b80ec9add3)

rust-timer · 2022-09-05T08:42:42Z

Queued 25a8c2d85ec7bbe4e5a7071f9725b3b80ec9add3 with parent 406e03f, future comparison URL.

fix `ExprKind` static_assert_size fix hir-stats

TaKO8Ki · 2022-09-05T14:12:42Z

@bors try @rust-timer queue

rust-timer · 2022-09-05T14:12:43Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

bors · 2022-09-05T14:12:51Z

⌛ Trying commit 9cde34e with merge 39e7cded5d29175e5dbf01353888be348eab6285...

rust-timer · 2022-09-05T15:34:20Z

Finished benchmarking commit (25a8c2d85ec7bbe4e5a7071f9725b3b80ec9add3): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.2%	[-1.3%, -1.1%]	2
Improvements ✅ (secondary)	-3.4%	[-3.9%, -3.0%]	6
All ❌✅ (primary)	-1.2%	[-1.3%, -1.1%]	2

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	2.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	2.4%	[1.2%, 5.2%]	14
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.9%	[-3.9%, -3.9%]	1
All ❌✅ (primary)	2.3%	[2.3%, 2.3%]	1

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.2%	[2.2%, 2.2%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

the arithmetic mean of the percent change ↩ ↩² ↩³
number of relevant changes ↩ ↩² ↩³

bors · 2022-09-05T15:50:56Z

☀️ Try build successful - checks-actions
Build commit: 39e7cded5d29175e5dbf01353888be348eab6285 (39e7cded5d29175e5dbf01353888be348eab6285)

cjgillot · 2022-09-05T16:02:11Z

@bors r+

bors · 2022-09-05T16:02:12Z

📌 Commit 9cde34e has been approved by cjgillot

It is now in the queue for this repository.

bors · 2022-09-05T16:21:43Z

⌛ Testing commit 9cde34e with merge b44197a...

bors · 2022-09-05T19:02:43Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing b44197a to master...

rust-timer · 2022-09-06T04:26:21Z

Finished benchmarking commit (b44197a): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.2%	[1.0%, 4.6%]	13
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.5%	[-4.3%, -2.6%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean¹	range	count²
Regressions ❌ (primary)	2.5%	[2.5%, 2.5%]	1
Regressions ❌ (secondary)	2.6%	[2.6%, 2.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.5%	[2.5%, 2.5%]	1

the arithmetic mean of the percent change ↩ ↩²
number of relevant changes ↩ ↩²

…ents-in-hir, r=cjgillot Separate the receiver from arguments in HIR Related to rust-lang#100232 cc `@cjgillot`

rustbot added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 1, 2022

rust-highfive assigned cjgillot Sep 1, 2022

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

TaKO8Ki force-pushed the separate-receiver-from-arguments-in-hir branch from 780f9ee to 7bf59f5 Compare September 1, 2022 04:29

This comment has been minimized.

Sign in to view

TaKO8Ki marked this pull request as draft September 1, 2022 07:40

This comment has been minimized.

Sign in to view

TaKO8Ki marked this pull request as ready for review September 1, 2022 10:55

deltragon reviewed Sep 1, 2022

View reviewed changes

compiler/rustc_hir/src/hir.rs Show resolved Hide resolved

cjgillot reviewed Sep 1, 2022

View reviewed changes

camsteffen reviewed Sep 1, 2022

View reviewed changes

cjgillot 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 2, 2022

TaKO8Ki force-pushed the separate-receiver-from-arguments-in-hir branch from 6f36cef to 38e5074 Compare September 2, 2022 13:49

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 2, 2022

TaKO8Ki requested a review from cjgillot September 3, 2022 04:23

cjgillot reviewed Sep 4, 2022

View reviewed changes

compiler/rustc_borrowck/src/diagnostics/region_errors.rs Outdated Show resolved Hide resolved

compiler/rustc_passes/src/liveness.rs Outdated Show resolved Hide resolved

src/tools/clippy/clippy_lints/src/eta_reduction.rs Outdated Show resolved Hide resolved

TaKO8Ki added 3 commits September 5, 2022 22:25

separate the receiver from arguments in HIR

87c6da3

separate the receiver from arguments in HIR under /clippy

3955dc3

refactor: remove unnecessary variables

fea1c5f

TaKO8Ki force-pushed the separate-receiver-from-arguments-in-hir branch 2 times, most recently from e6d951b to 1351d82 Compare September 5, 2022 13:49

use propagate_through_exprs instead of propagate_through_expr

9cde34e

fix `ExprKind` static_assert_size fix hir-stats

TaKO8Ki force-pushed the separate-receiver-from-arguments-in-hir branch from 1351d82 to 9cde34e Compare September 5, 2022 14:11

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 5, 2022

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 Sep 5, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 5, 2022

bors merged commit b44197a into rust-lang:master Sep 5, 2022

rustbot added this to the 1.65.0 milestone Sep 5, 2022

TaKO8Ki deleted the separate-receiver-from-arguments-in-hir branch September 5, 2022 19:18

TaKO8Ki mentioned this pull request Sep 5, 2022

Given how much better the AST is from having the receiver be separate, it makes me wonder if HIR would benefit just as much… #100427

Closed

bors mentioned this pull request Sep 5, 2022

Fix ICE, generalize 'move generics to trait' suggestion for >0 non-rcvr arguments #101427

Merged

flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 9, 2022

Auto merge of rust-lang#101261 - TaKO8Ki:separate-receiver-from-argum…

2ccf843

…ents-in-hir, r=cjgillot Separate the receiver from arguments in HIR Related to rust-lang#100232 cc `@cjgillot`

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate the receiver from arguments in HIR #101261

Separate the receiver from arguments in HIR #101261

TaKO8Ki commented Sep 1, 2022 •

edited

Loading

rustbot commented Sep 1, 2022

rust-highfive commented Sep 1, 2022

This comment has been minimized.

This comment has been minimized.

rustbot commented Sep 1, 2022

TaKO8Ki commented Sep 1, 2022

bors commented Sep 1, 2022

bors commented Sep 1, 2022

compiler-errors commented Sep 1, 2022

cjgillot Sep 1, 2022

camsteffen Sep 1, 2022

bors commented Sep 1, 2022

TaKO8Ki commented Sep 2, 2022

cjgillot left a comment

lcnr commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

TaKO8Ki commented Sep 5, 2022

rust-timer commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

bors commented Sep 5, 2022

cjgillot commented Sep 5, 2022

bors commented Sep 5, 2022

bors commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 6, 2022

Separate the receiver from arguments in HIR #101261

Separate the receiver from arguments in HIR #101261

Conversation

TaKO8Ki commented Sep 1, 2022 • edited Loading

rustbot commented Sep 1, 2022

rust-highfive commented Sep 1, 2022

This comment has been minimized.

This comment has been minimized.

rustbot commented Sep 1, 2022

TaKO8Ki commented Sep 1, 2022

bors commented Sep 1, 2022

bors commented Sep 1, 2022

compiler-errors commented Sep 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Sep 1, 2022

TaKO8Ki commented Sep 2, 2022

cjgillot left a comment

Choose a reason for hiding this comment

lcnr commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

TaKO8Ki commented Sep 5, 2022

rust-timer commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 5, 2022

Overall result: ✅ improvements - no action needed

Footnotes

bors commented Sep 5, 2022

cjgillot commented Sep 5, 2022

bors commented Sep 5, 2022

bors commented Sep 5, 2022

bors commented Sep 5, 2022

rust-timer commented Sep 6, 2022

Overall result: no relevant changes - no action needed

Footnotes

TaKO8Ki commented Sep 1, 2022 •

edited

Loading