Skip to content

Commit 25a3ea5

Browse files
add a new linting pass for used obligations
1 parent 4ca19e0 commit 25a3ea5

File tree

17 files changed

+263
-40
lines changed

17 files changed

+263
-40
lines changed

compiler/rustc_data_structures/src/obligation_forest/mod.rs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ pub trait ObligationProcessor {
102102
fn process_obligation(
103103
&mut self,
104104
obligation: &mut Self::Obligation,
105+
tracer: impl Fn() -> Vec<Self::Obligation>,
105106
) -> ProcessResult<Self::Obligation, Self::Error>;
106107

107108
/// As we do the cycle check, we invoke this callback when we
@@ -436,7 +437,11 @@ impl<O: ForestObligation> ObligationForest<O> {
436437
// be computed with the initial length, and we would miss the appended
437438
// nodes. Therefore we use a `while` loop.
438439
let mut index = 0;
439-
while let Some(node) = self.nodes.get_mut(index) {
440+
while index < self.nodes.len() {
441+
let (nodes, [node, ..]) = self.nodes.split_at_mut(index) else {
442+
todo!("FIXME(skippy)")
443+
};
444+
440445
// `processor.process_obligation` can modify the predicate within
441446
// `node.obligation`, and that predicate is the key used for
442447
// `self.active_cache`. This means that `self.active_cache` can get
@@ -447,7 +452,28 @@ impl<O: ForestObligation> ObligationForest<O> {
447452
continue;
448453
}
449454

450-
match processor.process_obligation(&mut node.obligation) {
455+
let parent_index = if node.has_parent { Some(node.dependents[0]) } else { None };
456+
let tracer = || {
457+
let mut trace = vec![];
458+
if let Some(mut index) = parent_index {
459+
loop {
460+
let node = &nodes[index];
461+
trace.push(node.obligation.clone());
462+
if node.has_parent {
463+
// The first dependent is the parent, which is treated
464+
// specially.
465+
index = node.dependents[0];
466+
} else {
467+
// No parent; treat all dependents non-specially.
468+
break;
469+
}
470+
}
471+
}
472+
473+
trace
474+
};
475+
476+
match processor.process_obligation(&mut node.obligation, tracer) {
451477
ProcessResult::Unchanged => {
452478
// No change in state.
453479
}

compiler/rustc_data_structures/src/obligation_forest/tests.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ where
7777
fn process_obligation(
7878
&mut self,
7979
obligation: &mut Self::Obligation,
80+
_tracer: impl Fn() -> Vec<Self::Obligation>,
8081
) -> ProcessResult<Self::Obligation, Self::Error> {
8182
(self.process_obligation)(obligation)
8283
}

compiler/rustc_infer/src/infer/at.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
7777
err_count_on_creation: self.err_count_on_creation,
7878
in_snapshot: self.in_snapshot.clone(),
7979
universe: self.universe.clone(),
80+
query_mode: self.query_mode.clone(),
8081
}
8182
}
8283
}

compiler/rustc_infer/src/infer/mod.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,21 @@ pub struct InferCtxt<'a, 'tcx> {
354354
/// when we enter into a higher-ranked (`for<..>`) type or trait
355355
/// bound.
356356
universe: Cell<ty::UniverseIndex>,
357+
358+
pub query_mode: TraitQueryMode,
359+
}
360+
361+
/// The mode that trait queries run in.
362+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
363+
pub enum TraitQueryMode {
364+
/// Standard/un-canonicalized queries get accurate
365+
/// spans etc. passed in and hence can do reasonable
366+
/// error reporting on their own.
367+
Standard,
368+
/// Canonicalized queries get dummy spans and hence
369+
/// must generally propagate errors to
370+
/// pre-canonicalization callsites.
371+
Canonical,
357372
}
358373

359374
/// See the `error_reporting` module for more details.
@@ -615,14 +630,26 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
615630
where
616631
T: TypeFoldable<'tcx>,
617632
{
618-
self.enter(|infcx| {
619-
let (value, subst) =
620-
infcx.instantiate_canonical_with_fresh_inference_vars(span, canonical);
621-
f(infcx, value, subst)
622-
})
633+
self.enter_with_query_mode(
634+
|infcx| {
635+
let (value, subst) =
636+
infcx.instantiate_canonical_with_fresh_inference_vars(span, canonical);
637+
f(infcx, value, subst)
638+
},
639+
TraitQueryMode::Canonical,
640+
)
623641
}
624642

625643
pub fn enter<R>(&mut self, f: impl for<'a> FnOnce(InferCtxt<'a, 'tcx>) -> R) -> R {
644+
self.enter_with_query_mode(f, TraitQueryMode::Standard)
645+
}
646+
647+
// FIXME(skippy) specifically needs reviewer feedback (see query_mode field)
648+
fn enter_with_query_mode<R>(
649+
&mut self,
650+
f: impl for<'a> FnOnce(InferCtxt<'a, 'tcx>) -> R,
651+
query_mode: TraitQueryMode,
652+
) -> R {
626653
let InferCtxtBuilder { tcx, defining_use_anchor, ref fresh_typeck_results } = *self;
627654
let in_progress_typeck_results = fresh_typeck_results.as_ref();
628655
f(InferCtxt {
@@ -640,6 +667,7 @@ impl<'tcx> InferCtxtBuilder<'tcx> {
640667
in_snapshot: Cell::new(false),
641668
skip_leak_check: Cell::new(false),
642669
universe: Cell::new(ty::UniverseIndex::ROOT),
670+
query_mode,
643671
})
644672
}
645673
}

compiler/rustc_trait_selection/src/traits/chalk_fulfill.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
33
use crate::infer::canonical::OriginalQueryValues;
44
use crate::infer::InferCtxt;
5+
use crate::traits::error_reporting::warnings::InferCtxtExt as _;
56
use crate::traits::query::NoSolution;
67
use crate::traits::{
78
ChalkEnvironmentAndGoal, FulfillmentError, FulfillmentErrorCode, ObligationCause,
@@ -109,12 +110,17 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
109110
&orig_values,
110111
&response,
111112
) {
112-
Ok(infer_ok) => next_round.extend(
113-
infer_ok.obligations.into_iter().map(|obligation| {
114-
assert!(!infcx.is_in_snapshot());
115-
infcx.resolve_vars_if_possible(obligation)
116-
}),
117-
),
113+
Ok(infer_ok) => {
114+
infcx.check_obligation_lints(&obligation, || {
115+
vec![obligation.clone()]
116+
});
117+
next_round.extend(infer_ok.obligations.into_iter().map(
118+
|obligation| {
119+
assert!(!infcx.is_in_snapshot());
120+
infcx.resolve_vars_if_possible(obligation)
121+
},
122+
))
123+
}
118124

119125
Err(_err) => errors.push(FulfillmentError {
120126
obligation: obligation.clone(),

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
pub mod on_unimplemented;
22
pub mod suggestions;
3+
pub mod warnings;
34

45
use super::{
56
DerivedObligationCause, EvaluationResult, FulfillmentContext, FulfillmentError,
@@ -2067,7 +2068,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'a, 'tcx> for InferCtxt<'a, 'tcx> {
20672068
);
20682069
let mut selcx = SelectionContext::with_query_mode(
20692070
&self,
2070-
crate::traits::TraitQueryMode::Standard,
2071+
crate::infer::TraitQueryMode::Standard,
20712072
);
20722073
match selcx.select_from_obligation(&obligation) {
20732074
Err(SelectionError::Ambiguous(impls)) if impls.len() > 1 => {
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
use super::PredicateObligation;
2+
3+
use crate::infer::InferCtxt;
4+
5+
use hir::{ItemKind, TyKind};
6+
use rustc_hir as hir;
7+
use rustc_hir::Node;
8+
use rustc_infer::infer::TraitQueryMode;
9+
use rustc_lint_defs::builtin::DEPRECATED_IN_FUTURE;
10+
use rustc_middle::ty::{self, Binder};
11+
12+
pub trait InferCtxtExt<'tcx> {
13+
fn check_obligation_lints(
14+
&self,
15+
obligation: &PredicateObligation<'tcx>,
16+
tracer: impl Fn() -> Vec<PredicateObligation<'tcx>>,
17+
);
18+
}
19+
20+
impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
21+
fn check_obligation_lints(
22+
&self,
23+
obligation: &PredicateObligation<'tcx>,
24+
tracer: impl Fn() -> Vec<PredicateObligation<'tcx>>,
25+
) {
26+
if self.query_mode != TraitQueryMode::Standard {
27+
return;
28+
}
29+
30+
if let Some(trait_obligation) = {
31+
let binder = obligation.predicate.kind();
32+
match binder.no_bound_vars() {
33+
None => match binder.skip_binder() {
34+
ty::PredicateKind::Trait(data) => Some(obligation.with(binder.rebind(data))),
35+
_ => None,
36+
},
37+
Some(pred) => match pred {
38+
ty::PredicateKind::Trait(data) => Some(obligation.with(Binder::dummy(data))),
39+
_ => None,
40+
},
41+
}
42+
} {
43+
let self_ty = trait_obligation.self_ty().skip_binder();
44+
let trait_ref = trait_obligation.predicate.skip_binder().trait_ref;
45+
46+
// FIXME(skippy) lint experiment: deprecate PartialEq on function pointers
47+
if let ty::FnPtr(_) = self_ty.kind()
48+
&& self.tcx.lang_items().eq_trait() == Some(trait_ref.def_id)
49+
{
50+
let node = self.tcx.hir().get(trait_obligation.cause.body_id);
51+
let mut skip = false;
52+
if let Node::Item(item) = node
53+
&& let ItemKind::Impl(impl_item) = &item.kind
54+
&& let TyKind::BareFn(_) = impl_item.self_ty.kind
55+
{
56+
skip = true;
57+
}
58+
if !skip {
59+
let root_obligation = tracer().last().unwrap_or(&obligation).clone();
60+
self.tcx.struct_span_lint_hir(
61+
DEPRECATED_IN_FUTURE,
62+
root_obligation.cause.body_id,
63+
root_obligation.cause.span,
64+
|lint| {
65+
lint.build(
66+
"FIXME(skippy) PartialEq on function pointers has been deprecated.",
67+
)
68+
.emit();
69+
},
70+
);
71+
}
72+
}
73+
}
74+
}
75+
}

compiler/rustc_trait_selection/src/traits/fulfill.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use super::Unimplemented;
2525
use super::{FulfillmentError, FulfillmentErrorCode};
2626
use super::{ObligationCause, PredicateObligation};
2727

28+
use crate::traits::error_reporting::warnings::InferCtxtExt as _;
2829
use crate::traits::error_reporting::InferCtxtExt as _;
2930
use crate::traits::project::PolyProjectionObligation;
3031
use crate::traits::project::ProjectionCacheKeyExt as _;
@@ -275,6 +276,7 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
275276
fn process_obligation(
276277
&mut self,
277278
pending_obligation: &mut Self::Obligation,
279+
tracer: impl Fn() -> Vec<Self::Obligation>,
278280
) -> ProcessResult<Self::Obligation, Self::Error> {
279281
// If we were stalled on some unresolved variables, first check whether
280282
// any of them have been resolved; if not, don't bother doing more work
@@ -313,7 +315,12 @@ impl<'a, 'b, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'tcx> {
313315
return ProcessResult::Unchanged;
314316
}
315317

316-
self.process_changed_obligations(pending_obligation)
318+
let result = self.process_changed_obligations(pending_obligation);
319+
if let ProcessResult::Changed(_) = result {
320+
let tracer = || tracer().into_iter().map(|x| x.obligation).collect();
321+
self.selcx.infcx().check_obligation_lints(&pending_obligation.obligation, tracer);
322+
}
323+
result
317324
}
318325

319326
fn process_backedge<'c, I>(

compiler/rustc_trait_selection/src/traits/mod.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -98,19 +98,6 @@ impl SkipLeakCheck {
9898
}
9999
}
100100

101-
/// The mode that trait queries run in.
102-
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
103-
pub enum TraitQueryMode {
104-
/// Standard/un-canonicalized queries get accurate
105-
/// spans etc. passed in and hence can do reasonable
106-
/// error reporting on their own.
107-
Standard,
108-
/// Canonicalized queries get dummy spans and hence
109-
/// must generally propagate errors to
110-
/// pre-canonicalization callsites.
111-
Canonical,
112-
}
113-
114101
/// Creates predicate obligations from the generic bounds.
115102
pub fn predicates_for_generics<'tcx>(
116103
cause: ObligationCause<'tcx>,

compiler/rustc_trait_selection/src/traits/query/evaluate_obligation.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
use rustc_middle::ty;
22

33
use crate::infer::canonical::OriginalQueryValues;
4-
use crate::infer::InferCtxt;
5-
use crate::traits::{
6-
EvaluationResult, OverflowError, PredicateObligation, SelectionContext, TraitQueryMode,
7-
};
4+
use crate::infer::{InferCtxt, TraitQueryMode};
5+
use crate::traits::{EvaluationResult, OverflowError, PredicateObligation, SelectionContext};
86

97
pub trait InferCtxtExt<'tcx> {
108
fn predicate_may_hold(&self, obligation: &PredicateObligation<'tcx>) -> bool;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ use super::wf;
1616
use super::{
1717
DerivedObligationCause, ErrorReporting, ImplDerivedObligation, ImplDerivedObligationCause,
1818
Normalized, Obligation, ObligationCause, ObligationCauseCode, Overflow, PredicateObligation,
19-
Selection, SelectionError, SelectionResult, TraitObligation, TraitQueryMode,
19+
Selection, SelectionError, SelectionResult, TraitObligation,
2020
};
2121

22-
use crate::infer::{InferCtxt, InferOk, TypeFreshener};
22+
use crate::infer::{InferCtxt, InferOk, TraitQueryMode, TypeFreshener};
2323
use crate::traits::error_reporting::InferCtxtExt;
2424
use crate::traits::project::ProjectAndUnifyResult;
2525
use crate::traits::project::ProjectionCacheKeyExt;

compiler/rustc_traits/src/evaluate_obligation.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ use rustc_infer::infer::TyCtxtInferExt;
22
use rustc_middle::ty::query::Providers;
33
use rustc_middle::ty::{ParamEnvAnd, TyCtxt};
44
use rustc_span::source_map::DUMMY_SP;
5+
use rustc_trait_selection::infer::TraitQueryMode;
56
use rustc_trait_selection::traits::query::CanonicalPredicateGoal;
67
use rustc_trait_selection::traits::{
7-
EvaluationResult, Obligation, ObligationCause, OverflowError, SelectionContext, TraitQueryMode,
8+
EvaluationResult, Obligation, ObligationCause, OverflowError, SelectionContext,
89
};
910

1011
crate fn provide(p: &mut Providers) {

compiler/rustc_typeck/src/check/coercion.rs

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ use rustc_session::parse::feature_err;
5858
use rustc_span::symbol::sym;
5959
use rustc_span::{self, BytePos, DesugaringKind, Span};
6060
use rustc_target::spec::abi::Abi;
61-
use rustc_trait_selection::traits::error_reporting::InferCtxtExt;
61+
use rustc_trait_selection::traits::error_reporting::warnings::InferCtxtExt as _;
62+
use rustc_trait_selection::traits::error_reporting::InferCtxtExt as _;
6263
use rustc_trait_selection::traits::{self, ObligationCause, ObligationCauseCode};
6364

6465
use smallvec::{smallvec, SmallVec};
@@ -604,15 +605,17 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
604605
// and almost never more than 3. By using a SmallVec we avoid an
605606
// allocation, at the (very small) cost of (occasionally) having to
606607
// shift subsequent elements down when removing the front element.
607-
let mut queue: SmallVec<[_; 4]> = smallvec![traits::predicate_for_trait_def(
608+
let root_obligation = traits::predicate_for_trait_def(
608609
self.tcx,
609610
self.fcx.param_env,
610611
cause,
611612
coerce_unsized_did,
612613
0,
613614
coerce_source,
614-
&[coerce_target.into()]
615-
)];
615+
&[coerce_target.into()],
616+
);
617+
// FIXME(skippy) extra .clone() here due to root_obligation
618+
let mut queue: SmallVec<[_; 4]> = smallvec![root_obligation.clone()];
616619

617620
let mut has_unsized_tuple_coercion = false;
618621
let mut has_trait_upcasting_coercion = false;
@@ -692,7 +695,11 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
692695
// be silent, as it causes a type mismatch later.
693696
}
694697

695-
Ok(Some(impl_source)) => queue.extend(impl_source.nested_obligations()),
698+
Ok(Some(impl_source)) => {
699+
// FIXME(skippy) just has the root_obligation, not the entire trace
700+
self.check_obligation_lints(&obligation, || vec![root_obligation.clone()]);
701+
queue.extend(impl_source.nested_obligations());
702+
}
696703
}
697704
}
698705

library/core/src/task/wake.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ impl RawWaker {
7373
/// any other `data` pointer will cause undefined behavior.
7474
#[stable(feature = "futures_api", since = "1.36.0")]
7575
#[derive(PartialEq, Copy, Clone, Debug)]
76+
// FIXME(skippy) for PartialEq on function pointers
77+
#[allow(deprecated_in_future)]
7678
pub struct RawWakerVTable {
7779
/// This function will be called when the [`RawWaker`] gets cloned, e.g. when
7880
/// the [`Waker`] in which the [`RawWaker`] is stored gets cloned.

0 commit comments

Comments
 (0)