Skip to content

Commit

Permalink
Point at specific field in struct literal when trait fulfillment fails
Browse files Browse the repository at this point in the history
  • Loading branch information
Nathan-Fenner committed Jan 23, 2023
1 parent c8e6a9e commit 2a67e99
Show file tree
Hide file tree
Showing 21 changed files with 1,151 additions and 112 deletions.
457 changes: 457 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/adjust_fulfillment_errors.rs

Large diffs are not rendered by default.

84 changes: 42 additions & 42 deletions compiler/rustc_hir_typeck/src/fn_ctxt/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,10 @@ use rustc_trait_selection::traits::{self, ObligationCauseCode, SelectionContext}

use std::iter;
use std::mem;
use std::ops::ControlFlow;
use std::slice;

use std::ops::ControlFlow;

impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
pub(in super::super) fn check_casts(&mut self) {
// don't hold the borrow to deferred_cast_checks while checking to avoid borrow checker errors
Expand Down Expand Up @@ -1837,7 +1838,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.into_iter()
.flatten()
{
if self.point_at_arg_if_possible(
if self.blame_specific_arg_if_possible(
error,
def_id,
param,
Expand Down Expand Up @@ -1867,7 +1868,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.into_iter()
.flatten()
{
if self.point_at_arg_if_possible(
if self.blame_specific_arg_if_possible(
error,
def_id,
param,
Expand All @@ -1892,16 +1893,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for param in
[param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
{
if let Some(param) = param
&& self.point_at_field_if_possible(
error,
if let Some(param) = param {
let refined_expr = self.point_at_field_if_possible(
def_id,
param,
variant_def_id,
fields,
)
{
return true;
);

match refined_expr {
None => {}
Some((refined_expr, _)) => {
error.obligation.cause.span = refined_expr
.span
.find_ancestor_in_same_ctxt(error.obligation.cause.span)
.unwrap_or(refined_expr.span);
return true;
}
}
}
}
}
Expand Down Expand Up @@ -1934,7 +1943,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

fn point_at_arg_if_possible(
/// - `blame_specific_*` means that the function will recursively traverse the expression,
/// looking for the most-specific-possible span to blame.
///
/// - `point_at_*` means that the function will only go "one level", pointing at the specific
/// expression mentioned.
///
/// `blame_specific_arg_if_possible` will find the most-specific expression anywhere inside
/// the provided function call expression, and mark it as responsible for the fullfillment
/// error.
fn blame_specific_arg_if_possible(
&self,
error: &mut traits::FulfillmentError<'tcx>,
def_id: DefId,
Expand All @@ -1953,13 +1971,20 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.inputs()
.iter()
.enumerate()
.filter(|(_, ty)| find_param_in_ty(**ty, param_to_point_at))
.filter(|(_, ty)| Self::find_param_in_ty((**ty).into(), param_to_point_at))
.collect();
// If there's one field that references the given generic, great!
if let [(idx, _)] = args_referencing_param.as_slice()
&& let Some(arg) = receiver
.map_or(args.get(*idx), |rcvr| if *idx == 0 { Some(rcvr) } else { args.get(*idx - 1) }) {

error.obligation.cause.span = arg.span.find_ancestor_in_same_ctxt(error.obligation.cause.span).unwrap_or(arg.span);

if let hir::Node::Expr(arg_expr) = self.tcx.hir().get(arg.hir_id) {
// This is more specific than pointing at the entire argument.
self.blame_specific_expr_if_possible(error, arg_expr)
}

error.obligation.cause.map_code(|parent_code| {
ObligationCauseCode::FunctionArgumentObligation {
arg_hir_id: arg.hir_id,
Expand All @@ -1977,14 +2002,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
false
}

fn point_at_field_if_possible(
// FIXME: Make this private and move to mod adjust_fulfillment_errors
pub fn point_at_field_if_possible(
&self,
error: &mut traits::FulfillmentError<'tcx>,
def_id: DefId,
param_to_point_at: ty::GenericArg<'tcx>,
variant_def_id: DefId,
expr_fields: &[hir::ExprField<'tcx>],
) -> bool {
) -> Option<(&'tcx hir::Expr<'tcx>, Ty<'tcx>)> {
let def = self.tcx.adt_def(def_id);

let identity_substs = ty::InternalSubsts::identity_for_item(self.tcx, def_id);
Expand All @@ -1994,7 +2019,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
.iter()
.filter(|field| {
let field_ty = field.ty(self.tcx, identity_substs);
find_param_in_ty(field_ty, param_to_point_at)
Self::find_param_in_ty(field_ty.into(), param_to_point_at)
})
.collect();

Expand All @@ -2004,17 +2029,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// same rules that check_expr_struct uses for macro hygiene.
if self.tcx.adjust_ident(expr_field.ident, variant_def_id) == field.ident(self.tcx)
{
error.obligation.cause.span = expr_field
.expr
.span
.find_ancestor_in_same_ctxt(error.obligation.cause.span)
.unwrap_or(expr_field.span);
return true;
return Some((expr_field.expr, self.tcx.type_of(field.did)));
}
}
}

false
None
}

fn point_at_path_if_possible(
Expand Down Expand Up @@ -2234,23 +2254,3 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
}

fn find_param_in_ty<'tcx>(ty: Ty<'tcx>, param_to_point_at: ty::GenericArg<'tcx>) -> bool {
let mut walk = ty.walk();
while let Some(arg) = walk.next() {
if arg == param_to_point_at {
return true;
} else if let ty::GenericArgKind::Type(ty) = arg.unpack()
&& let ty::Alias(ty::Projection, ..) = ty.kind()
{
// This logic may seem a bit strange, but typically when
// we have a projection type in a function signature, the
// argument that's being passed into that signature is
// not actually constraining that projection's substs in
// a meaningful way. So we skip it, and see improvements
// in some UI tests.
walk.skip_current_subtree();
}
}
false
}
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
mod _impl;
mod adjust_fulfillment_errors;
mod arg_matrix;
mod checks;
mod suggestions;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1568,6 +1568,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
traits::ImplDerivedObligationCause {
derived,
impl_def_id,
impl_def_predicate_index: None,
span,
},
))
Expand Down
50 changes: 26 additions & 24 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,30 +145,32 @@ impl<'tcx> Elaborator<'tcx> {
// Get predicates declared on the trait.
let predicates = tcx.super_predicates_of(data.def_id());

let obligations = predicates.predicates.iter().map(|&(mut pred, span)| {
// when parent predicate is non-const, elaborate it to non-const predicates.
if data.constness == ty::BoundConstness::NotConst {
pred = pred.without_const(tcx);
}

let cause = obligation.cause.clone().derived_cause(
bound_predicate.rebind(data),
|derived| {
traits::ImplDerivedObligation(Box::new(
traits::ImplDerivedObligationCause {
derived,
impl_def_id: data.def_id(),
span,
},
))
},
);
predicate_obligation(
pred.subst_supertrait(tcx, &bound_predicate.rebind(data.trait_ref)),
obligation.param_env,
cause,
)
});
let obligations =
predicates.predicates.iter().enumerate().map(|(index, &(mut pred, span))| {
// when parent predicate is non-const, elaborate it to non-const predicates.
if data.constness == ty::BoundConstness::NotConst {
pred = pred.without_const(tcx);
}

let cause = obligation.cause.clone().derived_cause(
bound_predicate.rebind(data),
|derived| {
traits::ImplDerivedObligation(Box::new(
traits::ImplDerivedObligationCause {
derived,
impl_def_id: data.def_id(),
impl_def_predicate_index: Some(index),
span,
},
))
},
);
predicate_obligation(
pred.subst_supertrait(tcx, &bound_predicate.rebind(data.trait_ref)),
obligation.param_env,
cause,
)
});
debug!(?data, ?obligations, "super_predicates");

// Only keep those bounds that we haven't already seen.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,8 @@ pub enum WellFormedLoc {
pub struct ImplDerivedObligationCause<'tcx> {
pub derived: DerivedObligationCause<'tcx>,
pub impl_def_id: DefId,
/// The index of the derived predicate in the parent impl's predicates.
pub impl_def_predicate_index: Option<usize>,
pub span: Span,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1224,6 +1224,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
ImplDerivedObligation(Box::new(ImplDerivedObligationCause {
derived,
impl_def_id,
impl_def_predicate_index: None,
span: obligation.cause.span,
}))
});
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2562,11 +2562,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
assert_eq!(predicates.parent, None);
let predicates = predicates.instantiate_own(tcx, substs);
let mut obligations = Vec::with_capacity(predicates.len());
for (predicate, span) in predicates {
for (index, (predicate, span)) in predicates.into_iter().enumerate() {
let cause = cause.clone().derived_cause(parent_trait_pred, |derived| {
ImplDerivedObligation(Box::new(ImplDerivedObligationCause {
derived,
impl_def_id: def_id,
impl_def_predicate_index: Some(index),
span,
}))
});
Expand Down
2 changes: 1 addition & 1 deletion src/doc/book
2 changes: 1 addition & 1 deletion src/doc/nomicon
2 changes: 1 addition & 1 deletion src/doc/rust-by-example
2 changes: 1 addition & 1 deletion src/tools/cargo
Submodule cargo updated 99 files
+7 −0 .github/workflows/contrib.yml
+0 −4 .github/workflows/main.yml
+5 −12 CHANGELOG.md
+4 −7 Cargo.toml
+0 −17 crates/cargo-test-macro/src/lib.rs
+1 −1 crates/cargo-test-support/Cargo.toml
+0 −1 crates/cargo-test-support/build.rs
+0 −26 crates/cargo-test-support/containers/apache/Dockerfile
+0 −4 crates/cargo-test-support/containers/apache/bar/Cargo.toml
+0 −1 crates/cargo-test-support/containers/apache/bar/src/lib.rs
+0 −12 crates/cargo-test-support/containers/apache/httpd-cargo.conf
+0 −29 crates/cargo-test-support/containers/sshd/Dockerfile
+0 −4 crates/cargo-test-support/containers/sshd/bar/Cargo.toml
+0 −1 crates/cargo-test-support/containers/sshd/bar/src/lib.rs
+0 −285 crates/cargo-test-support/src/containers.rs
+0 −7 crates/cargo-test-support/src/lib.rs
+1 −1 crates/cargo-test-support/src/registry.rs
+1 −1 crates/cargo-util/src/paths.rs
+1 −1 src/bin/cargo/commands/login.rs
+1 −2 src/bin/cargo/commands/owner.rs
+1 −3 src/bin/cargo/commands/publish.rs
+1 −2 src/bin/cargo/commands/yank.rs
+2 −42 src/cargo/core/compiler/artifact.rs
+3 −4 src/cargo/core/compiler/build_config.rs
+1 −1 src/cargo/core/compiler/build_context/target_info.rs
+41 −4 src/cargo/core/compiler/unit_dependencies.rs
+7 −14 src/cargo/core/dependency.rs
+2 −16 src/cargo/core/features.rs
+2 −1 src/cargo/core/package.rs
+3 −8 src/cargo/core/profiles.rs
+2 −2 src/cargo/lib.rs
+1 −1 src/cargo/ops/cargo_compile/compile_filter.rs
+1 −1 src/cargo/ops/cargo_compile/mod.rs
+3 −3 src/cargo/ops/cargo_compile/packages.rs
+2 −2 src/cargo/ops/cargo_compile/unit_generator.rs
+9 −16 src/cargo/ops/cargo_install.rs
+35 −134 src/cargo/ops/cargo_output_metadata.rs
+31 −35 src/cargo/ops/registry.rs
+17 −50 src/cargo/ops/vendor.rs
+0 −629 src/cargo/sources/git/known_hosts.rs
+0 −1 src/cargo/sources/git/mod.rs
+3 −31 src/cargo/sources/git/utils.rs
+3 −0 src/cargo/sources/registry/http_remote.rs
+1 −1 src/cargo/sources/registry/index.rs
+1 −1 src/cargo/sources/registry/mod.rs
+33 −147 src/cargo/util/auth.rs
+6 −101 src/cargo/util/config/de.rs
+18 −29 src/cargo/util/config/mod.rs
+1 −1 src/cargo/util/network.rs
+19 −49 src/cargo/util/toml/mod.rs
+0 −3 src/doc/contrib/book.toml
+0 −27 src/doc/contrib/src/tests/running.md
+0 −13 src/doc/contrib/src/tests/writing.md
+1 −1 src/doc/man/cargo-locate-project.md
+1 −1 src/doc/man/cargo-tree.md
+1 −1 src/doc/man/generated_txt/cargo-locate-project.txt
+1 −1 src/doc/man/generated_txt/cargo-tree.txt
+3 −10 src/doc/semver-check/src/main.rs
+0 −30 src/doc/src/appendix/git-authentication.md
+1 −1 src/doc/src/commands/cargo-locate-project.md
+1 −1 src/doc/src/commands/cargo-tree.md
+1 −5 src/doc/src/guide/cargo-home.md
+0 −53 src/doc/src/reference/config.md
+3 −23 src/doc/src/reference/registries.md
+5 −59 src/doc/src/reference/registry-index.md
+0 −55 src/doc/src/reference/semver.md
+31 −34 src/doc/src/reference/unstable.md
+1 −1 src/etc/man/cargo-locate-project.1
+1 −1 src/etc/man/cargo-tree.1
+3 −0 tests/testsuite/advanced_env.rs
+10 −4 tests/testsuite/alt_registry.rs
+6 −22 tests/testsuite/bad_config.rs
+1 −1 tests/testsuite/build.rs
+2 −2 tests/testsuite/cargo_add/dev_build_conflict/stderr.log
+3 −3 tests/testsuite/cargo_add/invalid_arg/stderr.log
+2 −2 tests/testsuite/cargo_add/invalid_target_empty/stderr.log
+2 −2 tests/testsuite/cargo_add/no_args/stderr.log
+1 −1 tests/testsuite/cargo_command.rs
+3 −3 tests/testsuite/cargo_remove/invalid_arg/stderr.log
+2 −2 tests/testsuite/cargo_remove/no_arg/stderr.log
+1 −57 tests/testsuite/config.rs
+1 −1 tests/testsuite/config_cli.rs
+0 −2 tests/testsuite/git.rs
+0 −152 tests/testsuite/https.rs
+3 −3 tests/testsuite/init/unknown_flags/stderr.log
+4 −42 tests/testsuite/install.rs
+17 −19 tests/testsuite/login.rs
+1 −1 tests/testsuite/logout.rs
+0 −2 tests/testsuite/main.rs
+4 −136 tests/testsuite/metadata.rs
+5 −3 tests/testsuite/new.rs
+0 −61 tests/testsuite/profile_config.rs
+23 −22 tests/testsuite/publish.rs
+271 −225 tests/testsuite/registry.rs
+12 −9 tests/testsuite/registry_auth.rs
+2 −5 tests/testsuite/run.rs
+1 −1 tests/testsuite/rustc.rs
+0 −548 tests/testsuite/ssh.rs
+2 −138 tests/testsuite/vendor.rs
24 changes: 12 additions & 12 deletions tests/ui/derives/deriving-copyclone.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0277]: the trait bound `B<C>: Copy` is not satisfied
--> $DIR/deriving-copyclone.rs:31:13
--> $DIR/deriving-copyclone.rs:31:26
|
LL | is_copy(B { a: 1, b: C });
| ------- ^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `B<C>`
| ------- ^ the trait `Copy` is not implemented for `B<C>`
| |
| required by a bound introduced by this call
|
Expand All @@ -19,14 +19,14 @@ LL | fn is_copy<T: Copy>(_: T) {}
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider borrowing here
|
LL | is_copy(&B { a: 1, b: C });
| +
LL | is_copy(B { a: 1, b: &C });
| +

error[E0277]: the trait bound `B<C>: Clone` is not satisfied
--> $DIR/deriving-copyclone.rs:32:14
--> $DIR/deriving-copyclone.rs:32:27
|
LL | is_clone(B { a: 1, b: C });
| -------- ^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `B<C>`
| -------- ^ the trait `Clone` is not implemented for `B<C>`
| |
| required by a bound introduced by this call
|
Expand All @@ -43,14 +43,14 @@ LL | fn is_clone<T: Clone>(_: T) {}
= note: this error originates in the derive macro `Clone` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider borrowing here
|
LL | is_clone(&B { a: 1, b: C });
| +
LL | is_clone(B { a: 1, b: &C });
| +

error[E0277]: the trait bound `B<D>: Copy` is not satisfied
--> $DIR/deriving-copyclone.rs:35:13
--> $DIR/deriving-copyclone.rs:35:26
|
LL | is_copy(B { a: 1, b: D });
| ------- ^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `B<D>`
| ------- ^ the trait `Copy` is not implemented for `B<D>`
| |
| required by a bound introduced by this call
|
Expand All @@ -67,8 +67,8 @@ LL | fn is_copy<T: Copy>(_: T) {}
= note: this error originates in the derive macro `Copy` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider borrowing here
|
LL | is_copy(&B { a: 1, b: D });
| +
LL | is_copy(B { a: 1, b: &D });
| +

error: aborting due to 3 previous errors

Expand Down
28 changes: 28 additions & 0 deletions tests/ui/errors/trait-bound-error-spans/blame-trait-error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
trait T1 {}
trait T2 {}
trait T3 {}
trait T4 {}

impl<B: T2> T1 for Wrapper<B> {}

impl T2 for i32 {}
impl T3 for i32 {}

impl<A: T3> T2 for Burrito<A> {}

struct Wrapper<W> {
value: W,
}

struct Burrito<F> {
filling: F,
}

fn want<V: T1>(_x: V) {}

fn example<Q>(q: Q) {
want(Wrapper { value: Burrito { filling: q } });
//~^ ERROR the trait bound `Q: T3` is not satisfied [E0277]
}

fn main() {}
Loading

0 comments on commit 2a67e99

Please sign in to comment.