Point at type parameter that introduced unmet bound instead of full HIR node #115219

estebank · 2023-08-25T16:22:41Z

error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
  --> $DIR/issue-87199.rs:18:15
   |
LL |     ref_arg::<[i32]>(&[5]);
   |               ^^^^^ doesn't have a size known at compile-time

instead of

error[E0277]: the size for values of type `[i32]` cannot be known at compilation time
  --> $DIR/issue-87199.rs:18:22
   |
LL |     ref_arg::<[i32]>(&[5]);
   |     ---------------- ^^^^ doesn't have a size known at compile-time
   |     |
   |     required by a bound introduced by this call

error[E0277]: the trait bound `String: Copy` is not satisfied
  --> $DIR/own-bound-span.rs:14:24
   |
LL |     let _: <S as D>::P<String>;
   |                        ^^^^^^ the trait `Copy` is not implemented for `String`
   |
note: required by a bound in `D::P`
  --> $DIR/own-bound-span.rs:4:15
   |
LL |     type P<T: Copy>;
   |               ^^^^ required by this bound in `D::P`

instead of

error[E0277]: the trait bound `String: Copy` is not satisfied
  --> $DIR/own-bound-span.rs:14:12
   |
LL |     let _: <S as D>::P<String>;
   |            ^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String`
   |
note: required by a bound in `D::P`
  --> $DIR/own-bound-span.rs:4:15
   |
LL |     type P<T: Copy>;
   |               ^^^^ required by this bound in `D::P`

Fix #105306.

rustbot · 2023-08-25T16:22:47Z

r? @b-naber

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

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs

compiler-errors

I'm somewhat tempted to ask if you can also split out the two main responsibilities of this PR, either into different PRs, or into different commits. My understanding is that it's doing two things:

Introducing variant_param_to_point_at is causing us to call point_at_generic_if_possible sooner than blame_specific_arg_if_possible. As I commented below, we don't need to add variant_param_to_point_at to do this -- just reorganizing the order of operation in the hir::ExprKind::Path(qpath) arm.
Registering additional WF obligations that have better cause codes via add_required_obligations_for_hir. This should be pretty ez to split out, but has nothing to do with the above afaict.

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs

compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs

compiler-errors · 2023-08-25T22:11:39Z

Registering redundant (but better cause-coded) obligations on bindings may have a perf implication, but it's probably thin. Worth testing now though. I wonder if we should do this somewhere more general though.

@bors try @rust-timer queue

bors · 2023-08-25T22:11:49Z

⌛ Trying commit c3d104657e284c71f963975e3c9c330943967111 with merge a0779a94a085d6466a9a3db3282a080353833847...

Walk through full path in `point_at_path_if_possible` We already had sufficient information to point at the `[u8]` in `Option::<[u8]>::None` (the `fallback_param_to_point_at` parameter), we just were neither using it nor walking through hir paths sufficiently to encounter it. This should alleviate the need to add additional logic to extract params in a somewhat arbitrary manner of looking at the grandparent def path: rust-lang#115219 (comment) r? `@estebank`

fmease · 2023-08-25T23:02:40Z

tests/ui/generic-associated-types/multiple-type-params-with-unmet-bounds.stderr

+LL |     let _: <() as Trait>::P<String, String>;
+   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String`
+   |
+note: required by a bound in `Trait::P`
+  --> $DIR/multiple-type-params-with-unmet-bounds.rs:2:15
+   |
+LL |     type P<T: Copy, U: Copy>;
+   |               ^^^^ required by this bound in `Trait::P`


I'm certain that you're aware that the output isn't correct yet but I'm leaving this comment here anyways to ensure it doesn't get forgotten ^^' Thanks for working on this btw!

Suggested change

LL | let _: <() as Trait>::P<String, String>;

| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `String`

|

note: required by a bound in `Trait::P`

--> $DIR/multiple-type-params-with-unmet-bounds.rs:2:15

|

LL | type P<T: Copy, U: Copy>;

| ^^^^ required by this bound in `Trait::P`

LL | let _: <() as Trait>::P<String, String>;

| ^^^^^^ the trait `Copy` is not implemented for `String`

|

note: required by a bound in `Trait::P`

--> $DIR/multiple-type-params-with-unmet-bounds.rs:2:15

|

LL | type P<T: Copy, U: Copy>;

| ^^^^ required by this bound in `Trait::P`

This is possibly a strange fallout of the way we do obligation deduplication in adjust_fulfillment_errors_for_expr_obligation. I'm inclined to say "don't worry about it", since it only triggers when you have duplicated bounds like this on the same item...

The second error is now gone from the removal of the select_where_possible call. I believe that we have some deduplication logic gone rogue that is silencing the trait bound error for the second parameter (likely because it looks like the same predicate?). I would like to fix that, but might have to be left as a future exercise.

I'm gonna open a separate issue for that later, basically uplifting #105306 (comment)

bors · 2023-08-25T23:21:55Z

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

On the following example, point at `String` instead of the whole type: ``` error[E0277]: the trait bound `String: Copy` is not satisfied --> $DIR/own-bound-span.rs:14:24 | LL | let _: <S as D>::P<String>; | ^^^^^^ the trait `Copy` is not implemented for `String` | note: required by a bound in `D::P` --> $DIR/own-bound-span.rs:4:15 | LL | type P<T: Copy>; | ^^^^ required by this bound in `D::P` ```

rust-timer · 2023-08-26T07:22:44Z

Finished benchmarking commit (a0779a94a085d6466a9a3db3282a080353833847): comparison URL.

Overall result: no relevant changes - 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-perf -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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.2%	[-2.2%, -2.2%]	1
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: 630.392s -> 630.853s (0.07%)
Artifact size: 315.69 MiB -> 315.59 MiB (-0.03%)

…errors

estebank · 2023-08-26T19:37:37Z

tests/ui/traits/inheritance/repeated-supertrait-ambig.stderr

 error[E0277]: the trait bound `dyn CompareToInts: CompareTo<i32>` is not satisfied
-  --> $DIR/repeated-supertrait-ambig.rs:34:37
+  --> $DIR/repeated-supertrait-ambig.rs:34:6
   |
 LL |     <dyn CompareToInts>::same_as(c, 22)
-   |     ----------------------------    ^^ the trait `CompareTo<i32>` is not implemented for `dyn CompareToInts`
-   |     |
-   |     required by a bound introduced by this call
+   |      ^^^^^^^^^^^^^^^^^ the trait `CompareTo<i32>` is not implemented for `dyn CompareToInts`
   |
   = help: the following other types implement trait `CompareTo<T>`:
             <i64 as CompareTo<i64>>


Pre-existing: we don't explain where CompareTo is coming from.

tests/ui/traits/dont-autoderef-ty-with-escaping-var.stderr

compiler-errors

r=me

estebank · 2023-08-26T21:09:45Z

@bors r=compiler-errors

bors · 2023-08-26T21:09:46Z

📌 Commit 7411e25 has been approved by compiler-errors

It is now in the queue for this repository.

bors · 2023-08-26T22:31:57Z

⌛ Testing commit 7411e25 with merge e877e2a...

bors · 2023-08-27T00:17:21Z

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

rust-timer · 2023-08-27T09:24:12Z

Finished benchmarking commit (e877e2a): 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)	4.2%	[4.1%, 4.2%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.1%, -1.1%]	1
All ❌✅ (primary)	-	-	0

Cycles

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

Binary size

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

Bootstrap: missing data
Artifact size: 316.12 MiB -> 316.05 MiB (-0.02%)

rustbot assigned b-naber Aug 25, 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. labels Aug 25, 2023

estebank force-pushed the issue-105306 branch from 47b7d15 to 8e3c434 Compare August 25, 2023 16:30

compiler-errors reviewed Aug 25, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Aug 25, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs Outdated Show resolved Hide resolved

compiler-errors reviewed Aug 25, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs Outdated Show resolved Hide resolved

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs Outdated Show resolved Hide resolved

compiler-errors mentioned this pull request Aug 25, 2023

Walk through full path in point_at_path_if_possible #115221

Merged

compiler-errors reviewed Aug 25, 2023

View reviewed changes

compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs Outdated Show resolved Hide resolved

compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 25, 2023

fmease reviewed Aug 25, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

Point at appropriate type parameter in more trait bound errors

120c24d

estebank force-pushed the issue-105306 branch from c3d1046 to 8f187fc Compare August 26, 2023 02:10

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 26, 2023

More accurately point at arguments

b6494a7

estebank force-pushed the issue-105306 branch from 8f187fc to b6494a7 Compare August 26, 2023 19:26

Remove unnecessary select_obligations_where_possible and redundant …

ef11db8

…errors

estebank commented Aug 26, 2023

View reviewed changes

tests/ui/traits/dont-autoderef-ty-with-escaping-var.stderr Show resolved Hide resolved

compiler-errors approved these changes Aug 26, 2023

View reviewed changes

Account for Weak alias kinds when adding more targetted obligation

7411e25

estebank force-pushed the issue-105306 branch from 24e374a to 7411e25 Compare August 26, 2023 20:10

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 Aug 26, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 27, 2023

bors merged commit e877e2a into rust-lang:master Aug 27, 2023

rustbot added this to the 1.74.0 milestone Aug 27, 2023

estebank deleted the issue-105306 branch November 9, 2023 05:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Point at type parameter that introduced unmet bound instead of full HIR node #115219

Point at type parameter that introduced unmet bound instead of full HIR node #115219

estebank commented Aug 25, 2023

rustbot commented Aug 25, 2023

compiler-errors left a comment

compiler-errors commented Aug 25, 2023

This comment has been minimized.

bors commented Aug 25, 2023

fmease Aug 25, 2023 •

edited

Loading

compiler-errors Aug 26, 2023

estebank Aug 26, 2023

fmease Aug 26, 2023 •

edited

Loading

bors commented Aug 25, 2023

This comment has been minimized.

rust-timer commented Aug 26, 2023

estebank Aug 26, 2023

compiler-errors left a comment

estebank commented Aug 26, 2023

bors commented Aug 26, 2023

bors commented Aug 26, 2023

bors commented Aug 27, 2023

rust-timer commented Aug 27, 2023

Point at type parameter that introduced unmet bound instead of full HIR node #115219

Point at type parameter that introduced unmet bound instead of full HIR node #115219

Conversation

estebank commented Aug 25, 2023

rustbot commented Aug 25, 2023

compiler-errors left a comment

Choose a reason for hiding this comment

compiler-errors commented Aug 25, 2023

This comment has been minimized.

bors commented Aug 25, 2023

fmease Aug 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmease Aug 26, 2023 • edited Loading

Choose a reason for hiding this comment

bors commented Aug 25, 2023

This comment has been minimized.

rust-timer commented Aug 26, 2023

Overall result: no relevant changes - no action needed

Choose a reason for hiding this comment

compiler-errors left a comment

Choose a reason for hiding this comment

estebank commented Aug 26, 2023

bors commented Aug 26, 2023

bors commented Aug 26, 2023

bors commented Aug 27, 2023

rust-timer commented Aug 27, 2023

Overall result: no relevant changes - no action needed

fmease Aug 25, 2023 •

edited

Loading

fmease Aug 26, 2023 •

edited

Loading