Implement special-cased projection error message for some common traits #98863

compiler-errors · 2022-07-03T20:20:28Z

Not sure what the best phrasing is, but I feel like these are more clear than the plain <Type as Iterator>::Output == Type messages.

If this is actually a good idea, are there any other traits this could benefit?

rust-highfive · 2022-07-03T20:20:31Z

r? @estebank

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

bors · 2022-07-08T06:29:54Z

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

estebank · 2022-08-04T14:59:59Z

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

+            ))
+        } else if Some(trait_def_id) == self.tcx.get_diagnostic_item(sym::Iterator) {
+            Some(format!(
+                "expected `{self_ty}` to be an iterator of `{expected_ty}`, but it actually returns items of `{normalized_ty}`"


I'm less enthusiastic about the wording "expected foo to be an iterator of i32 but it actually returns items of u32", because it feels we're mixing up concepts here. We could use "yields items of u32" instead, but yield isn't a keyword people will know from Rust (yet).

What about "expected foo to be an iterator over items of type i32 but it is an iterator over items of type u32"?

I think "yield" is okay here since it doesn't necessarily imply connection to the yield kw, but I can just use "over".

What do you think of

expected `foo` to be an iterator that yields `i32` but it yields `u32`

estebank · 2022-08-04T15:01:27Z

compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs

+            ))
+        } else if Some(trait_def_id) == self.tcx.lang_items().future_trait() {
+            Some(format!(
+                "expected `{self_ty}` to be a future that yields `{expected_ty}`, but it actually yields `{normalized_ty}`"


Maybe the same about using "yields" to talk about the Future::Output? What about "...to be a future that resolves to ty, but it resolves to ty".

Let's also drop the "actually". It doesn't add as much to the messaging, I feel.

estebank

The code changes look good, I have some qualms over wording. What do you think?

(Apologies for the review delay)

compiler-errors · 2022-08-04T16:42:31Z

I can apply these messaging changes, the ones I wrote originally were just kinda there to demonstrate the concept.

@rustbot author

compiler-errors · 2022-08-08T00:16:29Z

@rustbot ready

Adjusted the wording, what do you think now? Open to more changes if needed.

estebank · 2022-08-08T09:46:16Z

src/test/ui/hrtb/issue-62203-hrtb-ice.stderr

@@ -22,19 +22,22 @@ LL |     where
 LL |         F: for<'r> T0<'r, (<Self as Ty<'r>>::V,), O = <B as Ty<'r>>::V>,
   |                                                   ^^^^^^^^^^^^^^^^^^^^ required by this bound in `T1::m`

-error[E0271]: type mismatch resolving `for<'r> <[closure@$DIR/issue-62203-hrtb-ice.rs:42:17: 42:20] as FnOnce<((&'r u8,),)>>::Output == Unit3`
+error[E0271]: expected `[closure@$DIR/issue-62203-hrtb-ice.rs:42:16: 42:19]` to be a closure that returns `Unit3`, but it returns `Unit4`


Not for this PR: I would love to instead say "expected the closure to return Unit3..." and have a span label pointing at the closure, but we need to improve that in general.

estebank · 2022-08-08T09:47:15Z

src/test/ui/type-alias-impl-trait/issue-98608.stderr

@@ -1,7 +1,7 @@
-error[E0271]: type mismatch resolving `<fn() -> impl Sized {hi} as FnOnce<()>>::Output == Box<u8>`
-  --> $DIR/issue-98608.rs:4:39
+error[E0271]: expected `fn() -> impl Sized {hi}` to be a fn item that returns `Box<u8>`, but it returns `impl Sized`


Similarly here, we could refer to the function by name

expected `hi` to return `Box<u8>`, but it returns `impl Sized`

estebank · 2022-08-08T09:47:35Z

@bors r+

bors · 2022-08-08T09:47:37Z

📌 Commit 3fdf3cb has been approved by estebank

It is now in the queue for this repository.

bors · 2022-08-08T10:16:23Z

⌛ Testing commit 3fdf3cb with merge f03ce30...

bors · 2022-08-08T13:04:03Z

☀️ Test successful - checks-actions
Approved by: estebank
Pushing f03ce30 to master...

rust-timer · 2022-08-08T14:20:15Z

Finished benchmarking commit (f03ce30): comparison url.

Instruction count

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

Max RSS (memory usage)

Results

Primary benchmarks: no relevant changes found
Secondary benchmarks: mixed results

	mean¹	max	count²
Regressions 😿 (primary)	N/A	N/A	0
Regressions 😿 (secondary)	1.6%	1.6%	1
Improvements 🎉 (primary)	N/A	N/A	0
Improvements 🎉 (secondary)	-3.2%	-3.2%	1
All 😿🎉 (primary)	N/A	N/A	0

Cycles

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

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

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

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 3, 2022

rust-highfive assigned estebank Jul 3, 2022

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

compiler-errors force-pushed the projection-msg branch from 98197f5 to a1dcb95 Compare July 23, 2022 18:01

estebank reviewed Aug 4, 2022

View reviewed changes

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 Aug 4, 2022

compiler-errors added 2 commits August 7, 2022 23:57

Implement special-cased projection error message for some common traits

750f04d

Adjust wording

3fdf3cb

compiler-errors force-pushed the projection-msg branch from a1dcb95 to 3fdf3cb Compare August 8, 2022 00:13

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 Aug 8, 2022

estebank reviewed Aug 8, 2022

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 Aug 8, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 8, 2022

bors merged commit f03ce30 into rust-lang:master Aug 8, 2022

rustbot added this to the 1.65.0 milestone Aug 8, 2022

bors mentioned this pull request Aug 8, 2022

Print type of every call in a method call chain #96918

Closed

compiler-errors deleted the projection-msg branch August 11, 2023 20:14

lqd mentioned this pull request Aug 17, 2023

Soundness regression with for<'lt> closure having an implied bound from return type #114936

Closed

Jules-Bertholet mentioned this pull request Dec 12, 2023

&'a T → &'static T in safe code #118876

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement special-cased projection error message for some common traits #98863

Implement special-cased projection error message for some common traits #98863

compiler-errors commented Jul 3, 2022

rust-highfive commented Jul 3, 2022

bors commented Jul 8, 2022

estebank Aug 4, 2022

compiler-errors Aug 4, 2022

estebank Aug 4, 2022

estebank Aug 4, 2022

estebank left a comment •

edited

Loading

compiler-errors commented Aug 4, 2022

compiler-errors commented Aug 8, 2022

estebank Aug 8, 2022

estebank Aug 8, 2022

estebank commented Aug 8, 2022

bors commented Aug 8, 2022

bors commented Aug 8, 2022

bors commented Aug 8, 2022

rust-timer commented Aug 8, 2022

Implement special-cased projection error message for some common traits #98863

Implement special-cased projection error message for some common traits #98863

Conversation

compiler-errors commented Jul 3, 2022

rust-highfive commented Jul 3, 2022

bors commented Jul 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebank left a comment • edited Loading

Choose a reason for hiding this comment

compiler-errors commented Aug 4, 2022

compiler-errors commented Aug 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebank commented Aug 8, 2022

bors commented Aug 8, 2022

bors commented Aug 8, 2022

bors commented Aug 8, 2022

rust-timer commented Aug 8, 2022

Footnotes

estebank left a comment •

edited

Loading