Restore `const PartialEq` #118661

+    let args = args.into_iter().map(|arg| {
+        arg.into().unwrap_or_else(|| {
+            let orig = TypeVariableOrigin {
+                kind: TypeVariableOriginKind::MiscVariable,
+                span: DUMMY_SP,
+            };
+            infcx.next_ty_var(orig).into()
+        })
+    }).collect::<Vec<_>>();
+
+    // If an effect arg was not specified, we need to specify it.
+    let effect_arg = if tcx.generics_of(trait_id).host_effect_index.is_some_and(|x| args.get(x - 1).is_none()) {
+        Some(GenericArg::from(callee_id.map(|def_id| tcx.expected_host_effect_param_for_body(def_id)).unwrap_or(tcx.consts.true_)))
+    } else {
+        None
+    };


I think this is simplified by with_opt_host_effect_param

I don't think that works. There are times where the caller of this function has already specified the host param themselves (they're modifying an existing GenericArg list) and there's args.get(x-1) (since args is without the Self type) to check for that.

compiler-errors

Please amend that last commit with addressing nit, then r=me

fee1-dead · 2023-12-11T00:52:47Z

with_opt_host_effect_param always appends a host param if there is one, but that's not what we want to do here, so I don't think it can be simplified.

@bors r=compiler-errors

bors · 2023-12-11T00:52:49Z

📌 Commit c4c3555 has been approved by compiler-errors

It is now in the queue for this repository.

…rtialEq, r=compiler-errors Restore `const PartialEq` And thus fixes a number of tests. There is a bug that still needs to be fixed, so WIP for now. r? `@compiler-errors`

…rtialEq, r=compiler-errors Restore `const PartialEq` And thus fixes a number of tests. There is a bug that still needs to be fixed, so WIP for now. r? ``@compiler-errors``

…llaumeGomez Rollup of 5 pull requests Successful merges: - rust-lang#118417 (Add unstable `-Zdefault-hidden-visibility` cmdline flag for `rustc`.) - rust-lang#118661 (Restore `const PartialEq`) - rust-lang#118802 (Remove edition umbrella features.) - rust-lang#118807 (Remove an allocation in min_stack) - rust-lang#118812 (rustdoc-search: do not treat associated type names as types) r? `@ghost` `@rustbot` modify labels: rollup

bors · 2023-12-11T10:34:54Z

⌛ Testing commit c4c3555 with merge 6f40082...

bors · 2023-12-11T12:31:55Z

☀️ Test successful - checks-actions
Approved by: compiler-errors
Pushing 6f40082 to master...

rust-timer · 2023-12-11T13:52:16Z

Finished benchmarking commit (6f40082): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.3%	[0.2%, 0.4%]	9
Regressions ❌ (secondary)	0.3%	[0.2%, 0.6%]	11
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-0.8%, -0.8%]	1
All ❌✅ (primary)	0.3%	[0.2%, 0.4%]	9

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)	1.8%	[1.4%, 2.2%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.5%	[-6.3%, -0.7%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.9%	[-6.3%, 2.2%]	4

Cycles

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

Binary size

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.0%	[0.0%, 0.0%]	1
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	6
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.0%	[0.0%, 0.0%]	1

Bootstrap: 669.313s -> 668.867s (-0.07%)
Artifact size: 314.17 MiB -> 314.18 MiB (0.00%)

fee1-dead · 2023-12-12T13:57:38Z

The type_of query is being run more here? I think that may be because PartialEq now has a new generic parameter which causes rustc to do analysis. Something that we're probably bound to take, but is probably small compared to the gains at 1 2 3 when we took apart the logic before the redesign.

…ialEq, r=compiler-errors Restore `const PartialEq` And thus fixes a number of tests. There is a bug that still needs to be fixed, so WIP for now. r? `@compiler-errors`

smoelius · 2024-01-11T15:16:28Z

src/tools/clippy/clippy_utils/src/ty.rs

@@ -223,9 +224,10 @@ pub fn implements_trait_with_env<'tcx>(
    param_env: ParamEnv<'tcx>,
    ty: Ty<'tcx>,
    trait_id: DefId,
+    callee_id: DefId,


@fee1-dead (cc: @compiler-errors) I am having difficulty understanding what this additional required callee_id parameter is/does.

A comment elsewhere in this PR suggests it is an "effect arg". Moreover, I found this doc comment for with_opt_host_effect_param:

rust/compiler/rustc_middle/src/ty/util.rs

Line 860 in d73bd3f

/// Constructs generic args for an item, optionally appending a const effect param type

But I can't find any documentation (e.g., in the Rustc Dev Guide) on what an "effect arg/param" is, or why/when it is needed.

Could you please point me to something that could help me better understand? Thank you.

I've hidden the last comment since it could be wrong. I will take a look maybe tomorrow.

Ensure `callee_id`s are body owners This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner. rust-lang#118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations. Frankly, I [do not completely understand](rust-lang#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner: - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372 In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that. cc `@rust-lang/clippy` r? `@fee1-dead`

Ensure `callee_id`s are body owners This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner. rust-lang#118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations. Frankly, I [do not completely understand](rust-lang#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner: - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372 In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that. cc ``@rust-lang/clippy`` r? ``@fee1-dead``

Rollup merge of rust-lang#120000 - smoelius:fix-clippy, r=fee1-dead Ensure `callee_id`s are body owners This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner. rust-lang#118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations. Frankly, I [do not completely understand](rust-lang#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner: - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372 In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that. cc ``@rust-lang/clippy`` r? ``@fee1-dead``

Ensure `callee_id`s are body owners This PR makes the `callee_id` argument of Clippy's `implements_trait_with_env` optional, and when it is passed, ensures it is a body owner. #118661 added the `callee_id` parameter to alleviate an ICE. Specifically, the `callee_id` is used to determine an "effect arg" in certain situations. Frankly, I [do not completely understand](rust-lang/rust#118661 (comment)) what an "effect arg" is. But the code that determines it seems to require that `callee_id` is a body owner: - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/src/tools/clippy/clippy_utils/src/ty.rs#L286-L288 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/ty/util.rs#L834 - https://github.com/rust-lang/rust/blob/1ead4761e9e2f056385768614c23ffa7acb6a19e/compiler/rustc_middle/src/hir/map/mod.rs#L372 In the current head, some def ids passed as `callee_id`s are not body owners. This PR fixes that. cc ``@rust-lang/clippy`` r? ``@fee1-dead``

rustbot assigned compiler-errors Dec 6, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 6, 2023

This comment has been minimized.

Sign in to view

rustbot 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 Dec 8, 2023

fee1-dead added 5 commits December 10, 2023 09:30

Restore const PartialEq

f635cd2

fix host param for PartialEq

e03c18b

filter out trailing effect param in own_args_no_defaults

2f457d9

Revert "Don't print host effect param in pretty path_generic_args"

d1f4bc5

This reverts commit f1bf874.

fix tests

d464dd0

fee1-dead force-pushed the restore-const-partialEq branch from 268d5f3 to d464dd0 Compare December 10, 2023 10:42

fee1-dead marked this pull request as ready for review December 10, 2023 10:42

clean up leftover FIXME

de8f4ac

fee1-dead 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 Dec 10, 2023

rustbot 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 Dec 10, 2023

fix clippy

c4c3555

fee1-dead force-pushed the restore-const-partialEq branch from 65ae516 to c4c3555 Compare December 10, 2023 13:11

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 Dec 10, 2023

compiler-errors reviewed Dec 10, 2023

View reviewed changes

compiler-errors approved these changes Dec 10, 2023

View reviewed changes

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 Dec 11, 2023

GuillaumeGomez mentioned this pull request Dec 11, 2023

Rollup of 4 pull requests #118820

Closed

GuillaumeGomez mentioned this pull request Dec 11, 2023

Rollup of 5 pull requests #118821

Closed

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 11, 2023

bors merged commit 6f40082 into rust-lang:master Dec 11, 2023

rustbot added this to the 1.76.0 milestone Dec 11, 2023

rustbot added the perf-regression Performance regression. label Dec 11, 2023

fee1-dead deleted the restore-const-partialEq branch December 12, 2023 13:32

smoelius reviewed Jan 11, 2024

View reviewed changes

smoelius mentioned this pull request Jan 15, 2024

Ensure callee_ids are body owners #120000

Merged

fee1-dead mentioned this pull request Feb 1, 2024

Reconstify Add #120381

Merged

Nadrieril mentioned this pull request Jun 14, 2024

Update rustc cryspen/hax#721

Merged

Restore const PartialEq #118661

Restore const PartialEq #118661

Uh oh!

Conversation

fee1-dead commented Dec 6, 2023

Uh oh!

This comment has been minimized.

compiler-errors commented Dec 8, 2023

Uh oh!

rustbot commented Dec 10, 2023

Uh oh!

fee1-dead commented Dec 10, 2023

Uh oh!

compiler-errors commented Dec 10, 2023

Uh oh!

rustbot commented Dec 10, 2023

Uh oh!

fee1-dead commented Dec 10, 2023

Uh oh!

compiler-errors Dec 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fee1-dead Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors Dec 11, 2023

Choose a reason for hiding this comment

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

fee1-dead commented Dec 11, 2023

Uh oh!

bors commented Dec 11, 2023

Uh oh!

bors commented Dec 11, 2023

Uh oh!

bors commented Dec 11, 2023

Uh oh!

rust-timer commented Dec 11, 2023

Overall result: ❌ regressions - ACTION NEEDED

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

fee1-dead commented Dec 12, 2023

Uh oh!

smoelius Jan 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

fee1-dead Jan 11, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Restore `const PartialEq` #118661

Restore `const PartialEq` #118661

compiler-errors Dec 10, 2023 •

edited

Loading

smoelius Jan 11, 2024 •

edited

Loading