-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Point out the known type when field doesn't satisfy bound #38150
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
let (post_message, pre_message) = | ||
if let ObligationCauseCode::BuiltinDerivedObligation(ref data) | ||
= obligation.cause.code | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This brace should be on the previous line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
For file ```rust use std::path::Path; fn f(p: Path) { } ``` provide the following error ```nocode error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path` --> file.rs:3:6 | 3 | fn f(p: Path) { } | ^ within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]` | = note: `[u8]` does not have a constant size known at compile-time = note: required because it appears within the type `std::path::Path` = note: all local variables must have a statically known size ```
let (post_message, pre_message) = | ||
if let ObligationCauseCode::BuiltinDerivedObligation(ref data) | ||
= obligation.cause.code { | ||
let parent_trait_ref = self.resolve_type_vars_if_possible( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing an indent level here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it? the if let
is part of the previous line, the only reason is below is because I couldn't get to fit nicely in under 100 chars. In this were to fit in one line it would be:
let (post_message, pre_message) = if let ObligationCauseCode::BuiltinDerivedObligation(ref data) = obligation.cause.code {
I don't see why then line 493 would have to have double indentation. :)
I agree that it is slightly confusing as it is now, but haven't come up with a better way of presenting this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh indeed, you're absolutely right, my bad!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one better way might be using match
:)
let (post_message, pre_message) = match obligation.cause.code {
ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
let parent_trait_ref = self.resolve_type_vars_if_possible(
...
}
_ => {
(String::new(), String::new())
}
};
All good for me. I'm not sure I'm supposed to merge this so I'll wait for others' opinion. |
let (post_message, pre_message) = | ||
if let ObligationCauseCode::BuiltinDerivedObligation(ref data) | ||
= obligation.cause.code { | ||
let parent_trait_ref = self.resolve_type_vars_if_possible( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one better way might be using match
:)
let (post_message, pre_message) = match obligation.cause.code {
ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
let parent_trait_ref = self.resolve_type_vars_if_possible(
...
}
_ => {
(String::new(), String::new())
}
};
= obligation.cause.code { | ||
let parent_trait_ref = self.resolve_type_vars_if_possible( | ||
&data.parent_trait_ref); | ||
(format!(" in `{}`", parent_trait_ref.0.self_ty()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One concern I have is that this only goes up one level, but there are similar cases where multiple levels are needed. For example, it'd be nice if this patch addressed this example: https://is.gd/BZjfP6
The current output is
error[E0277]: the trait bound `*const u8: std::marker::Send` is not satisfied
--> <anon>:16:5
|
16 | is_send::<Foo>();
| ^^^^^^^^^^^^^^ trait `*const u8: std::marker::Send` not satisfied
|
= note: `*const u8` cannot be sent between threads safely
= note: required because it appears within the type `Baz`
= note: required because it appears within the type `Bar`
= note: required because it appears within the type `Foo`
= note: required by `is_send`
and I think your suggested approach would be a big improvement:
error[E0277]: within `Foo`, the trait bound `*const u8: std::marker::Send` is not satisfied
--> <anon>:16:5
|
16 | is_send::<Foo>();
| ^^^^^^^^^^^^^^ trait `*const u8: std::marker::Send` not satisfied
|
= note: `*const u8` cannot be sent between threads safely
= note: required because it appears within the type `Baz`
= note: required because it appears within the type `Bar`
= note: required because it appears within the type `Foo`
= note: required by `is_send`
But I believe your patch as is will print "within Baz
" instead.
should be easy enough though -- just have to iterate on the cause chain rather than just going up one level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added test for new case and a recursive search up the cause chain.
@bors r+ |
📌 Commit 5c13041 has been approved by |
⌛ Testing commit 5c13041 with merge 27122ab... |
💔 Test failed - auto-win-gnu-64-opt |
@bors: retry
…On Mon, Dec 19, 2016 at 6:11 PM, bors ***@***.***> wrote:
💔 Test failed - auto-win-gnu-64-opt
<https://buildbot.rust-lang.org/builders/auto-win-gnu-64-opt/builds/6460>
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#38150 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAD95LptLW_NJMW6wRoRfGy0H5S-uZ07ks5rJzlkgaJpZM4LDcVM>
.
|
Point out the known type when field doesn't satisfy bound For file ```rust use std::path::Path; fn f(p: Path) { } ``` provide the following error ```nocode error[E0277]: the trait bound `[u8]: std::marker::Sized` is not satisfied in `std::path::Path` --> file.rs:3:6 | 3 | fn f(p: Path) { } | ^ within `std::path::Path`, the trait `std::marker::Sized` is not implemented for `[u8]` | = note: `[u8]` does not have a constant size known at compile-time = note: required because it appears within the type `std::path::Path` = note: all local variables must have a statically known size ``` Fix rust-lang#23286.
Rollup of 29 pull requests - Successful merges: #37761, #38006, #38131, #38150, #38158, #38171, #38208, #38215, #38236, #38245, #38289, #38302, #38315, #38346, #38388, #38395, #38398, #38418, #38432, #38451, #38463, #38468, #38470, #38471, #38472, #38478, #38486, #38493, #38498 - Failed merges: #38271, #38483
For file
provide the following error
Fix #23286.