Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

E0277 should use short paths when possible #116616

Closed
estebank opened this issue Oct 10, 2023 · 8 comments · Fixed by #116739
Closed

E0277 should use short paths when possible #116616

estebank opened this issue Oct 10, 2023 · 8 comments · Fixed by #116739
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-papercut Diagnostics: An error or lint that needs small tweaks. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

estebank commented Oct 10, 2023

Type errors (and others) use tcx.short_ty_string and similar to write types that are too long to disk, so the cli output is easier to read. Trait bound errors do not do that.

https://twitter.com/ceccon_me/status/1707741832824734186

@estebank estebank added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. D-papercut Diagnostics: An error or lint that needs small tweaks. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. labels Oct 10, 2023
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 10, 2023
@fracek
Copy link

fracek commented Oct 11, 2023

I created a small repo to reproduce this error message. What was not visible in the tweet but that you will see when you build the repo, is that the types in the error notes are even longer. That's a shame because hidden in that noise the compiler gives the solution to the error (pin).

@Vedaant-Rajoo
Copy link

@fracek You can just create a gist with the error generating logic line highlighted. Much more helpful?

@fracek
Copy link

fracek commented Oct 11, 2023

I can give more context too. I was implementing a K8s operator using kube.

I start by creating a Controller and then running it with .run. This returns a a stream with a relatively complex type.

type ReconcileItem<K> = Result<(ObjectRef<K>, Action), controller::Error<MyError, watcher::Error>>;

async fn create_controller(
    client: Client,
) -> Result<impl Stream<Item = ReconcileItem<Pod>>, MyError> {
    let pods = Api::<Pod>::all(client.clone());
    let context = Context { client };

    let controller = Controller::new(pods, watcher::Config::default()).run(
        reconcile,
        error_policy,
        context.into(),
    );

    Ok(controller)
}

That stream yields an item every time the reconcile loops reconciles an item or returns an error. For testing, I iterate over the stream to check that it reconciles items the way I expect.

    let mut controller = create_controller(client).await?;
    while let Some(_item) = controller.try_next().await? { // <-- this line generates the error
    }

This is the error I get compiling that piece of code.

The solution is to pin the stream returned by create_controller.

@fmease fmease removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 11, 2023
@Milo123459
Copy link
Contributor

how would i go about solving this?

@rustbot claim

@estebank
Copy link
Contributor Author

@Milo123459 you can look at how we do it here

let (self_ty, file) =
self.tcx.short_ty_string(parent_trait_pred.skip_binder().self_ty());

You have to do the same thing but at the very least in the places we print out types to string here:

let trait_predicate = bound_predicate.rebind(trait_predicate);
let trait_predicate = self.resolve_vars_if_possible(trait_predicate);
// FIXME(effects)
let predicate_is_const = false;
if self.tcx.sess.has_errors().is_some()
&& trait_predicate.references_error()
{
return;
}
let trait_ref = trait_predicate.to_poly_trait_ref();
let (post_message, pre_message, type_def) = self
.get_parent_trait_ref(obligation.cause.code())
.map(|(t, s)| {
(
format!(" in `{t}`"),
format!("within `{t}`, "),
s.map(|s| (format!("within this `{t}`"), s)),
)
})
.unwrap_or_default();
let OnUnimplementedNote {
message,
label,
note,
parent_label,
append_const_msg,
} = self.on_unimplemented_note(trait_ref, &obligation);
let have_alt_message = message.is_some() || label.is_some();
let is_try_conversion = self.is_try_conversion(span, trait_ref.def_id());
let is_unsize =
Some(trait_ref.def_id()) == self.tcx.lang_items().unsize_trait();
let (message, note, append_const_msg) = if is_try_conversion {
(
Some(format!(
"`?` couldn't convert the error to `{}`",
trait_ref.skip_binder().self_ty(),
)),
Some(
"the question mark operation (`?`) implicitly performs a \
conversion on the error value using the `From` trait"
.to_owned(),
),
Some(AppendConstMessage::Default),
)
} else {
(message, note, append_const_msg)
};
let err_msg = self.get_standard_error_message(
&trait_predicate,
message,
predicate_is_const,
append_const_msg,
post_message,
);

@Vedaant-Rajoo
Copy link

You also need to ensure that the type-shortening is conditional, based on whether the user wants verbose output or not (similar to the existing logic for type errors).

@estebank
Copy link
Contributor Author

@Vedaant-Rajoo short_ty_string already checks whether write_long_types_to_disk is enabled or not before shortening.

@Milo123459

This comment was marked as outdated.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Oct 28, 2023
@bors bors closed this as completed in 471e33f Oct 28, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 28, 2023
Rollup merge of rust-lang#116739 - Milo123459:milo/short-paths, r=estebank

Make `E0277` use short paths

Fixes rust-lang#116616
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints D-papercut Diagnostics: An error or lint that needs small tweaks. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants