Skip to content

Commit 6848ba2

Browse files
committedAug 21, 2022
Comment a bit, use find_ancestor_in_same_ctxt to suppress some weird macro notes
1 parent 292ab39 commit 6848ba2

File tree

5 files changed

+151
-100
lines changed

5 files changed

+151
-100
lines changed
 

‎compiler/rustc_span/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,13 @@ impl Span {
664664
Some(self)
665665
}
666666

667+
pub fn find_ancestor_in_same_ctxt(mut self, other: Span) -> Option<Span> {
668+
while !Span::eq_ctxt(self, other) {
669+
self = self.parent_callsite()?;
670+
}
671+
Some(self)
672+
}
673+
667674
/// Edition of the crate from which this span came.
668675
pub fn edition(self) -> edition::Edition {
669676
self.ctxt().edition()

‎compiler/rustc_typeck/src/check/fn_ctxt/checks.rs

Lines changed: 144 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
657657
Applicability::MachineApplicable,
658658
);
659659
};
660-
self.label_fn_like(&mut err, fn_def_id, callee_ty, Some(mismatch_idx), is_method);
660+
self.label_fn_like(
661+
&mut err,
662+
fn_def_id,
663+
callee_ty,
664+
Some(mismatch_idx),
665+
is_method,
666+
);
661667
err.emit();
662668
return;
663669
}
@@ -1064,8 +1070,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10641070
}
10651071
let suggestion_text = if let Some(provided_idx) = provided_idx
10661072
&& let (_, provided_span) = provided_arg_tys[*provided_idx]
1067-
&& let Ok(arg_text) =
1068-
source_map.span_to_snippet(provided_span)
1073+
&& let Ok(arg_text) = source_map.span_to_snippet(provided_span)
10691074
{
10701075
arg_text
10711076
} else {
@@ -1603,22 +1608,32 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16031608
}
16041609
}
16051610

1606-
/// Given a vec of evaluated `FulfillmentError`s and an `fn` call argument expressions, we walk
1607-
/// the checked and coerced types for each argument to see if any of the `FulfillmentError`s
1608-
/// reference a type argument. The reason to walk also the checked type is that the coerced type
1609-
/// can be not easily comparable with predicate type (because of coercion). If the types match
1610-
/// for either checked or coerced type, and there's only *one* argument that does, we point at
1611-
/// the corresponding argument's expression span instead of the `fn` call path span.
1611+
/// Given a vector of fulfillment errors, try to adjust the spans of the
1612+
/// errors to more accurately point at the cause of the failure.
1613+
///
1614+
/// This applies to calls, methods, and struct expressions. This will also
1615+
/// try to deduplicate errors that are due to the same cause but might
1616+
/// have been created with different [`ObligationCause`][traits::ObligationCause]s.
16121617
pub(super) fn adjust_fulfillment_errors_for_expr_obligation(
16131618
&self,
16141619
errors: &mut Vec<traits::FulfillmentError<'tcx>>,
16151620
) {
1621+
// Store a mapping from `(Span, Predicate) -> ObligationCause`, so that
1622+
// other errors that have the same span and predicate can also get fixed,
1623+
// even if their `ObligationCauseCode` isn't an `Expr*Obligation` kind.
1624+
// This is important since if we adjust one span but not the other, then
1625+
// we will have "duplicated" the error on the UI side.
16161626
let mut remap_cause = FxHashSet::default();
16171627
let mut not_adjusted = vec![];
16181628

16191629
for error in errors {
16201630
let before_span = error.obligation.cause.span;
1621-
if self.adjust_fulfillment_error_for_expr_obligation(error) {
1631+
if self.adjust_fulfillment_error_for_expr_obligation(error)
1632+
|| before_span != error.obligation.cause.span
1633+
{
1634+
// Store both the predicate and the predicate *without constness*
1635+
// since sometimes we instantiate and check both of these in a
1636+
// method call, for example.
16221637
remap_cause.insert((
16231638
before_span,
16241639
error.obligation.predicate,
@@ -1630,6 +1645,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16301645
error.obligation.cause.clone(),
16311646
));
16321647
} else {
1648+
// If it failed to be adjusted once around, it may be adjusted
1649+
// via the "remap cause" mapping the second time...
16331650
not_adjusted.push(error);
16341651
}
16351652
}
@@ -1697,7 +1714,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
16971714
};
16981715

16991716
// Prefer generics that are local to the fn item, since these are likely
1700-
// to be the cause of the unsatisfied predicaete.
1717+
// to be the cause of the unsatisfied predicate.
17011718
let mut param_to_point_at = find_param_matching(&|param_ty| {
17021719
self.tcx.parent(generics.type_param(param_ty, self.tcx).def_id) == def_id
17031720
});
@@ -1708,14 +1725,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17081725
&& param_ty.name != rustc_span::symbol::kw::SelfUpper
17091726
});
17101727
// Finally, the `Self` parameter is possibly the reason that the predicate
1711-
// is unsatisfied. This is less likely to be true for methods, because the
1728+
// is unsatisfied. This is less likely to be true for methods, because
17121729
// method probe means that we already kinda check that the predicates due
17131730
// to the `Self` type are true.
17141731
let mut self_param_to_point_at =
17151732
find_param_matching(&|param_ty| param_ty.name == rustc_span::symbol::kw::SelfUpper);
17161733

1717-
// For ambiguity errors, we actually want to look for a parameter that is
1718-
// the source of the inference type left over in this predicate.
1734+
// Finally, for ambiguity-related errors, we actually want to look
1735+
// for a parameter that is the source of the inference type left
1736+
// over in this predicate.
17191737
if let traits::FulfillmentErrorCode::CodeAmbiguity = error.code {
17201738
fallback_param_to_point_at = None;
17211739
self_param_to_point_at = None;
@@ -1724,25 +1742,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17241742
}
17251743

17261744
let hir = self.tcx.hir();
1727-
17281745
match hir.get(hir_id) {
17291746
hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Path(qpath), hir_id, .. }) => {
1730-
if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Call(callee, args), hir_id: call_hir_id, .. })
1731-
= hir.get(hir.get_parent_node(*hir_id))
1747+
if let hir::Node::Expr(hir::Expr {
1748+
kind: hir::ExprKind::Call(callee, args),
1749+
hir_id: call_hir_id,
1750+
..
1751+
}) = hir.get(hir.get_parent_node(*hir_id))
17321752
&& callee.hir_id == *hir_id
17331753
{
1734-
for param in [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at] {
1754+
for param in
1755+
[param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
1756+
{
17351757
if let Some(param) = param
1736-
&& self.point_at_arg_if_possible(error, def_id, param, *call_hir_id, callee.span, args)
1758+
&& self.point_at_arg_if_possible(
1759+
error,
1760+
def_id,
1761+
param,
1762+
*call_hir_id,
1763+
callee.span,
1764+
args,
1765+
)
17371766
{
17381767
return true;
17391768
}
17401769
}
1741-
17421770
// Notably, we only point to params that are local to the
17431771
// item we're checking, since those are the ones we are able
1744-
// to look in the hir::PathSegment for. Everything else
1745-
// would require a deeper search into the qpath than I think
1772+
// to look in the final `hir::PathSegment` for. Everything else
1773+
// would require a deeper search into the `qpath` than I think
17461774
// is worthwhile.
17471775
if let Some(param_to_point_at) = param_to_point_at
17481776
&& self.point_at_path_if_possible(error, def_id, param_to_point_at, qpath)
@@ -1758,12 +1786,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17581786
for param in [param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
17591787
{
17601788
if let Some(param) = param
1761-
&& self.point_at_arg_if_possible(error, def_id, param, hir_id, segment.ident.span, args)
1789+
&& self.point_at_arg_if_possible(
1790+
error,
1791+
def_id,
1792+
param,
1793+
hir_id,
1794+
segment.ident.span,
1795+
args,
1796+
)
17621797
{
17631798
return true;
17641799
}
17651800
}
1766-
17671801
if let Some(param_to_point_at) = param_to_point_at
17681802
&& self.point_at_generic_if_possible(error, def_id, param_to_point_at, segment)
17691803
{
@@ -1780,13 +1814,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17801814
[param_to_point_at, fallback_param_to_point_at, self_param_to_point_at]
17811815
{
17821816
if let Some(param) = param
1783-
&& self.point_at_field_if_possible(error, def_id, param, variant_def_id, fields)
1817+
&& self.point_at_field_if_possible(
1818+
error,
1819+
def_id,
1820+
param,
1821+
variant_def_id,
1822+
fields,
1823+
)
17841824
{
17851825
return true;
17861826
}
17871827
}
17881828
}
1789-
17901829
if let Some(param_to_point_at) = param_to_point_at
17911830
&& self.point_at_path_if_possible(error, def_id, param_to_point_at, qpath)
17921831
{
@@ -1799,32 +1838,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
17991838
false
18001839
}
18011840

1802-
fn point_at_path_if_possible(
1803-
&self,
1804-
error: &mut traits::FulfillmentError<'tcx>,
1805-
def_id: DefId,
1806-
param: ty::GenericArg<'tcx>,
1807-
qpath: &QPath<'tcx>,
1808-
) -> bool {
1809-
match qpath {
1810-
hir::QPath::Resolved(_, path) => {
1811-
if let Some(segment) = path.segments.last()
1812-
&& self.point_at_generic_if_possible(error, def_id, param, segment)
1813-
{
1814-
return true;
1815-
}
1816-
}
1817-
hir::QPath::TypeRelative(_, segment) => {
1818-
if self.point_at_generic_if_possible(error, def_id, param, segment) {
1819-
return true;
1820-
}
1821-
}
1822-
_ => {}
1823-
}
1824-
1825-
false
1826-
}
1827-
18281841
fn point_at_arg_if_possible(
18291842
&self,
18301843
error: &mut traits::FulfillmentError<'tcx>,
@@ -1839,29 +1852,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18391852
.inputs()
18401853
.iter()
18411854
.enumerate()
1842-
.filter(|(_, ty)| find_param_in_ty(ty, param_to_point_at))
1855+
.filter(|(_, ty)| find_param_in_ty(**ty, param_to_point_at))
18431856
.collect();
18441857

1845-
if let [(idx, _)] = args_referencing_param.as_slice()
1846-
&& let Some(arg) = args.get(*idx)
1847-
{
1848-
error.obligation.cause.span = arg.span;
1858+
// If there's one field that references the given generic, great!
1859+
if let [(idx, _)] = args_referencing_param.as_slice() && let Some(arg) = args.get(*idx) {
1860+
error.obligation.cause.span = arg.span.find_ancestor_in_same_ctxt(error.obligation.cause.span).unwrap_or(arg.span);
18491861
error.obligation.cause.map_code(|parent_code| {
18501862
ObligationCauseCode::FunctionArgumentObligation {
18511863
arg_hir_id: arg.hir_id,
18521864
call_hir_id,
18531865
parent_code,
18541866
}
18551867
});
1856-
true
1868+
return true;
18571869
} else if args_referencing_param.len() > 0 {
1858-
// If more than one argument applies, then point to the callee
1870+
// If more than one argument applies, then point to the callee span at least...
18591871
// We have chance to fix this up further in `point_at_generics_if_possible`
18601872
error.obligation.cause.span = callee_span;
1861-
false
1862-
} else {
1863-
false
18641873
}
1874+
1875+
false
18651876
}
18661877

18671878
fn point_at_field_if_possible(
@@ -1873,30 +1884,60 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
18731884
expr_fields: &[hir::ExprField<'tcx>],
18741885
) -> bool {
18751886
let def = self.tcx.adt_def(def_id);
1887+
18761888
let identity_substs = ty::InternalSubsts::identity_for_item(self.tcx, def_id);
18771889
let fields_referencing_param: Vec<_> = def
18781890
.variant_with_id(variant_def_id)
18791891
.fields
18801892
.iter()
18811893
.filter(|field| {
18821894
let field_ty = field.ty(self.tcx, identity_substs);
1883-
match find_param_in_ty(field_ty, param_to_point_at) {
1884-
Ok(value) => value,
1885-
Err(value) => return value,
1886-
}
1895+
find_param_in_ty(field_ty, param_to_point_at)
18871896
})
18881897
.collect();
1898+
18891899
if let [field] = fields_referencing_param.as_slice() {
18901900
for expr_field in expr_fields {
1901+
// Look for the ExprField that matches the field, using the
1902+
// same rules that check_expr_struct uses for macro hygiene.
18911903
if self.tcx.adjust_ident(expr_field.ident, variant_def_id) == field.ident(self.tcx)
18921904
{
1893-
error.obligation.cause.span = expr_field.span;
1905+
error.obligation.cause.span = expr_field
1906+
.span
1907+
.find_ancestor_in_same_ctxt(error.obligation.cause.span)
1908+
.unwrap_or(expr_field.span);
1909+
return true;
18941910
}
18951911
}
1896-
true
1897-
} else {
1898-
false
18991912
}
1913+
1914+
false
1915+
}
1916+
1917+
fn point_at_path_if_possible(
1918+
&self,
1919+
error: &mut traits::FulfillmentError<'tcx>,
1920+
def_id: DefId,
1921+
param: ty::GenericArg<'tcx>,
1922+
qpath: &QPath<'tcx>,
1923+
) -> bool {
1924+
match qpath {
1925+
hir::QPath::Resolved(_, path) => {
1926+
if let Some(segment) = path.segments.last()
1927+
&& self.point_at_generic_if_possible(error, def_id, param, segment)
1928+
{
1929+
return true;
1930+
}
1931+
}
1932+
hir::QPath::TypeRelative(_, segment) => {
1933+
if self.point_at_generic_if_possible(error, def_id, param, segment) {
1934+
return true;
1935+
}
1936+
}
1937+
_ => {}
1938+
}
1939+
1940+
false
19001941
}
19011942

19021943
fn point_at_generic_if_possible(
@@ -1921,7 +1962,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
19211962
.iter()
19221963
.filter(|arg| matches!(arg, hir::GenericArg::Type(_)))
19231964
.nth(index) else { return false; };
1924-
error.obligation.cause.span = arg.span();
1965+
error.obligation.cause.span = arg
1966+
.span()
1967+
.find_ancestor_in_same_ctxt(error.obligation.cause.span)
1968+
.unwrap_or(arg.span());
19251969
true
19261970
}
19271971

@@ -1935,11 +1979,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
19351979
type BreakTy = ty::GenericArg<'tcx>;
19361980
fn visit_ty(&mut self, ty: Ty<'tcx>) -> std::ops::ControlFlow<Self::BreakTy> {
19371981
if let Some(origin) = self.0.type_var_origin(ty)
1938-
&& let TypeVariableOriginKind::TypeParameterDefinition(_, Some(def_id))
1939-
= origin.kind
1982+
&& let TypeVariableOriginKind::TypeParameterDefinition(_, Some(def_id)) =
1983+
origin.kind
19401984
&& let generics = self.0.tcx.generics_of(self.1)
19411985
&& let Some(index) = generics.param_def_id_to_index(self.0.tcx, def_id)
1942-
&& let Some(subst) = ty::InternalSubsts::identity_for_item(self.0.tcx, self.1).get(index as usize)
1986+
&& let Some(subst) = ty::InternalSubsts::identity_for_item(self.0.tcx, self.1)
1987+
.get(index as usize)
19431988
{
19441989
ControlFlow::Break(*subst)
19451990
} else {
@@ -2015,14 +2060,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
20152060
let new_def_id = self.probe(|_| {
20162061
let trait_ref = ty::TraitRef::new(
20172062
call_kind.to_def_id(self.tcx),
2018-
self.tcx.mk_substs([
2019-
ty::GenericArg::from(callee_ty),
2020-
self.next_ty_var(TypeVariableOrigin {
2021-
kind: TypeVariableOriginKind::MiscVariable,
2022-
span: rustc_span::DUMMY_SP,
2023-
})
2024-
.into(),
2025-
].into_iter()),
2063+
self.tcx.mk_substs(
2064+
[
2065+
ty::GenericArg::from(callee_ty),
2066+
self.next_ty_var(TypeVariableOrigin {
2067+
kind: TypeVariableOriginKind::MiscVariable,
2068+
span: rustc_span::DUMMY_SP,
2069+
})
2070+
.into(),
2071+
]
2072+
.into_iter(),
2073+
),
20262074
);
20272075
let obligation = traits::Obligation::new(
20282076
traits::ObligationCause::dummy(),
@@ -2037,7 +2085,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
20372085
Ok(Some(traits::ImplSource::UserDefined(impl_source))) => {
20382086
Some(impl_source.impl_def_id)
20392087
}
2040-
_ => None
2088+
_ => None,
20412089
}
20422090
});
20432091
if let Some(new_def_id) = new_def_id {
@@ -2092,22 +2140,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
20922140
}
20932141
}
20942142

2095-
fn find_param_in_ty(ty: Ty, param_to_point_at: ty::GenericArg) -> bool {
2143+
fn find_param_in_ty<'tcx>(ty: Ty<'tcx>, param_to_point_at: ty::GenericArg<'tcx>) -> bool {
20962144
let mut walk = ty.walk();
20972145
while let Some(arg) = walk.next() {
20982146
if arg == param_to_point_at {
2099-
return true;
2100-
} else if let ty::GenericArgKind::Type(ty) = arg.unpack()
2101-
&& let ty::Projection(..) = ty.kind()
2102-
{
2103-
// This logic may seem a bit strange, but typically when
2104-
// we have a projection type in a function signature, the
2105-
// argument that's being passed into that signature is
2106-
// not actually constraining that projection's substs in
2107-
// a meaningful way. So we skip it, and see improvements
2108-
// in some UI tests.
2109-
walk.skip_current_subtree();
2110-
}
2147+
return true;
2148+
} else if let ty::GenericArgKind::Type(ty) = arg.unpack()
2149+
&& let ty::Projection(..) = ty.kind()
2150+
{
2151+
// This logic may seem a bit strange, but typically when
2152+
// we have a projection type in a function signature, the
2153+
// argument that's being passed into that signature is
2154+
// not actually constraining that projection's substs in
2155+
// a meaningful way. So we skip it, and see improvements
2156+
// in some UI tests.
2157+
walk.skip_current_subtree();
2158+
}
21112159
}
21122160
false
21132161
}

‎src/test/ui/fmt/send-sync.stderr

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ note: required by a bound in `send`
1717
|
1818
LL | fn send<T: Send>(_: T) {}
1919
| ^^^^ required by this bound in `send`
20-
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
2120

2221
error[E0277]: `core::fmt::Opaque` cannot be shared between threads safely
2322
--> $DIR/send-sync.rs:9:10
@@ -38,7 +37,6 @@ note: required by a bound in `sync`
3837
|
3938
LL | fn sync<T: Sync>(_: T) {}
4039
| ^^^^ required by this bound in `sync`
41-
= note: this error originates in the macro `format_args` (in Nightly builds, run with -Z macro-backtrace for more info)
4240

4341
error: aborting due to 2 previous errors
4442

‎src/test/ui/issues/issue-60218.stderr

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ LL | pub fn trigger_error<I, F>(iterable: I, functor: F)
1414
...
1515
LL | for<'t> <Map<<&'t I as IntoIterator>::IntoIter, F> as Iterator>::Item: Foo,
1616
| ^^^ required by this bound in `trigger_error`
17-
= note: this error originates in the macro `vec` (in Nightly builds, run with -Z macro-backtrace for more info)
1817

1918
error: aborting due to previous error
2019

‎src/test/ui/never_type/feature-gate-never_type_fallback.stderr

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ note: required by a bound in `foo`
1313
|
1414
LL | fn foo(_: impl T) {}
1515
| ^ required by this bound in `foo`
16-
= note: this error originates in the macro `$crate::panic::panic_2015` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
1716

1817
error: aborting due to previous error
1918

0 commit comments

Comments
 (0)
Please sign in to comment.