Skip to content

Commit b1bf35a

Browse files
nnethercotennethercote
committed
Eliminate ObligationCauseData.
PR #72962 shrank `Obligation` at the cost of more heap allocations; overall it was a perf win. This PR partly undoes that change, making `Obligation` a little bigger (though not as big as it was) while reducing the number of heap allocations.
1 parent c977b87 commit b1bf35a

File tree

17 files changed

+76
-76
lines changed

17 files changed

+76
-76
lines changed

‎src/librustc_infer/infer/error_reporting/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
585585
cause: &ObligationCause<'tcx>,
586586
exp_found: Option<ty::error::ExpectedFound<Ty<'tcx>>>,
587587
) {
588-
match cause.code {
588+
match *cause.code() {
589589
ObligationCauseCode::Pattern { origin_expr: true, span: Some(span), root_ty } => {
590590
let ty = self.resolve_vars_if_possible(&root_ty);
591591
if ty.is_suggestable() {
@@ -2058,7 +2058,7 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
20582058
fn as_failure_code(&self, terr: &TypeError<'tcx>) -> FailureCode {
20592059
use self::FailureCode::*;
20602060
use crate::traits::ObligationCauseCode::*;
2061-
match self.code {
2061+
match self.code() {
20622062
CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"),
20632063
CompareImplTypeObligation { .. } => Error0308("type not compatible with trait"),
20642064
MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => {
@@ -2097,7 +2097,7 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> {
20972097

20982098
fn as_requirement_str(&self) -> &'static str {
20992099
use crate::traits::ObligationCauseCode::*;
2100-
match self.code {
2100+
match self.code() {
21012101
CompareImplMethodObligation { .. } => "method type is compatible with trait",
21022102
CompareImplTypeObligation { .. } => "associated type is compatible with trait",
21032103
ExprAssignable => "expression is assignable",

‎src/librustc_infer/infer/error_reporting/nice_region_error/placeholder_error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ impl NiceRegionError<'me, 'tcx> {
222222
format!("trait `{}` defined here", self.tcx().def_path_str(trait_def_id)),
223223
);
224224

225-
let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id) = cause.code {
225+
let leading_ellipsis = if let ObligationCauseCode::ItemObligation(def_id) = *cause.code() {
226226
err.span_label(span, "doesn't satisfy where-clause");
227227
err.span_label(
228228
self.tcx().def_span(def_id),

‎src/librustc_infer/infer/error_reporting/nice_region_error/trait_impl_difference.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ impl<'a, 'tcx> NiceRegionError<'a, 'tcx> {
3434
ValuePairs::Types(sub_expected_found),
3535
ValuePairs::Types(sup_expected_found),
3636
CompareImplMethodObligation { trait_item_def_id, .. },
37-
) = (&sub_trace.values, &sup_trace.values, &sub_trace.cause.code)
37+
) = (&sub_trace.values, &sup_trace.values, sub_trace.cause.code())
3838
{
3939
if sup_expected_found == sub_expected_found {
4040
self.emit_err(

‎src/librustc_infer/infer/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1774,7 +1774,7 @@ impl<'tcx> SubregionOrigin<'tcx> {
17741774
where
17751775
F: FnOnce() -> Self,
17761776
{
1777-
match cause.code {
1777+
match *cause.code() {
17781778
traits::ObligationCauseCode::ReferenceOutlivesReferent(ref_type) => {
17791779
SubregionOrigin::ReferenceOutlivesReferent(ref_type, cause.span)
17801780
}

‎src/librustc_infer/traits/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub type TraitObligation<'tcx> = Obligation<'tcx, ty::PolyTraitPredicate<'tcx>>;
5757

5858
// `PredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger.
5959
#[cfg(target_arch = "x86_64")]
60-
static_assert_size!(PredicateObligation<'_>, 48);
60+
static_assert_size!(PredicateObligation<'_>, 64);
6161

6262
pub type Obligations<'tcx, O> = Vec<Obligation<'tcx, O>>;
6363
pub type PredicateObligations<'tcx> = Vec<PredicateObligation<'tcx>>;

‎src/librustc_middle/traits/mod.rs

Lines changed: 32 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ use rustc_span::{Span, DUMMY_SP};
2020
use smallvec::SmallVec;
2121

2222
use std::borrow::Cow;
23-
use std::fmt;
24-
use std::ops::Deref;
23+
//use std::fmt; // njn: temp
2524
use std::rc::Rc;
2625

2726
pub use self::select::{EvaluationCache, EvaluationResult, OverflowError, SelectionCache};
@@ -81,38 +80,14 @@ pub enum Reveal {
8180

8281
/// The reason why we incurred this obligation; used for error reporting.
8382
///
84-
/// As the happy path does not care about this struct, storing this on the heap
85-
/// ends up increasing performance.
83+
/// Non-dummy `ObligationCauseCode`s are stored on the heap. This gives best
84+
/// trade-off between keeping the type small (which makes copies cheaper) while
85+
/// not doing too many heap allocations.
8686
///
8787
/// We do not want to intern this as there are a lot of obligation causes which
8888
/// only live for a short period of time.
89-
#[derive(Clone, PartialEq, Eq, Hash)]
90-
pub struct ObligationCause<'tcx> {
91-
/// `None` for `ObligationCause::dummy`, `Some` otherwise.
92-
data: Option<Rc<ObligationCauseData<'tcx>>>,
93-
}
94-
95-
const DUMMY_OBLIGATION_CAUSE_DATA: ObligationCauseData<'static> =
96-
ObligationCauseData { span: DUMMY_SP, body_id: hir::CRATE_HIR_ID, code: MiscObligation };
97-
98-
// Correctly format `ObligationCause::dummy`.
99-
impl<'tcx> fmt::Debug for ObligationCause<'tcx> {
100-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
101-
ObligationCauseData::fmt(self, f)
102-
}
103-
}
104-
105-
impl Deref for ObligationCause<'tcx> {
106-
type Target = ObligationCauseData<'tcx>;
107-
108-
#[inline(always)]
109-
fn deref(&self) -> &Self::Target {
110-
self.data.as_deref().unwrap_or(&DUMMY_OBLIGATION_CAUSE_DATA)
111-
}
112-
}
113-
11489
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
115-
pub struct ObligationCauseData<'tcx> {
90+
pub struct ObligationCause<'tcx> {
11691
pub span: Span,
11792

11893
/// The ID of the fn body that triggered this obligation. This is
@@ -123,17 +98,38 @@ pub struct ObligationCauseData<'tcx> {
12398
/// information.
12499
pub body_id: hir::HirId,
125100

126-
pub code: ObligationCauseCode<'tcx>,
101+
/// `None` for `DUMMY_OBLIGATION_CAUSE_CODE` (a very common case), `Some`
102+
/// otherwise.
103+
code: Option<Rc<ObligationCauseCode<'tcx>>>,
127104
}
128105

106+
const DUMMY_OBLIGATION_CAUSE_CODE: ObligationCauseCode<'static> = MiscObligation;
107+
108+
// Correctly format `DUMMY_OBLIGATION_CAUSE_CODE`.
109+
// njn: fix this
110+
//impl<'tcx> fmt::Debug for ObligationCauseData<'tcx> {
111+
// fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
112+
// ObligationCauseCode::fmt(&self.code2, f)
113+
// }
114+
//}
115+
129116
impl<'tcx> ObligationCause<'tcx> {
117+
#[inline(always)]
118+
pub fn code(&self) -> &ObligationCauseCode<'tcx> {
119+
self.code.as_deref().unwrap_or(&DUMMY_OBLIGATION_CAUSE_CODE)
120+
}
121+
130122
#[inline]
131123
pub fn new(
132124
span: Span,
133125
body_id: hir::HirId,
134126
code: ObligationCauseCode<'tcx>,
135127
) -> ObligationCause<'tcx> {
136-
ObligationCause { data: Some(Rc::new(ObligationCauseData { span, body_id, code })) }
128+
if code == DUMMY_OBLIGATION_CAUSE_CODE {
129+
ObligationCause { span, body_id, code: None }
130+
} else {
131+
ObligationCause { span, body_id, code: Some(Rc::new(code)) }
132+
}
137133
}
138134

139135
pub fn misc(span: Span, body_id: hir::HirId) -> ObligationCause<'tcx> {
@@ -146,15 +142,15 @@ impl<'tcx> ObligationCause<'tcx> {
146142

147143
#[inline(always)]
148144
pub fn dummy() -> ObligationCause<'tcx> {
149-
ObligationCause { data: None }
145+
ObligationCause { span: DUMMY_SP, body_id: hir::CRATE_HIR_ID, code: None }
150146
}
151147

152-
pub fn make_mut(&mut self) -> &mut ObligationCauseData<'tcx> {
153-
Rc::make_mut(self.data.get_or_insert_with(|| Rc::new(DUMMY_OBLIGATION_CAUSE_DATA)))
148+
pub fn make_mut_code(&mut self) -> &mut ObligationCauseCode<'tcx> {
149+
Rc::make_mut(self.code.get_or_insert_with(|| Rc::new(DUMMY_OBLIGATION_CAUSE_CODE)))
154150
}
155151

156152
pub fn span(&self, tcx: TyCtxt<'tcx>) -> Span {
157-
match self.code {
153+
match *self.code() {
158154
ObligationCauseCode::CompareImplMethodObligation { .. }
159155
| ObligationCauseCode::MainFunctionType
160156
| ObligationCauseCode::StartFunctionType => {

‎src/librustc_middle/traits/structural_impls.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,8 @@ impl<'a, 'tcx> Lift<'tcx> for traits::DerivedObligationCause<'a> {
232232
impl<'a, 'tcx> Lift<'tcx> for traits::ObligationCause<'a> {
233233
type Lifted = traits::ObligationCause<'tcx>;
234234
fn lift_to_tcx(&self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
235-
tcx.lift(&self.code).map(|code| traits::ObligationCause::new(self.span, self.body_id, code))
235+
tcx.lift(self.code())
236+
.map(|code| traits::ObligationCause::new(self.span, self.body_id, code))
236237
}
237238
}
238239

‎src/librustc_middle/ty/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ impl<T> Trait<T> for X {
495495
proj_ty,
496496
values,
497497
body_owner_def_id,
498-
&cause.code,
498+
cause.code(),
499499
);
500500
}
501501
(_, ty::Projection(proj_ty)) => {

‎src/librustc_trait_selection/traits/error_reporting/mod.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
198198
self.note_obligation_cause_code(
199199
&mut err,
200200
&obligation.predicate,
201-
&obligation.cause.code,
201+
obligation.cause.code(),
202202
&mut vec![],
203203
);
204204

@@ -242,7 +242,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
242242
item_name,
243243
impl_item_def_id,
244244
trait_item_def_id,
245-
} = obligation.cause.code
245+
} = *obligation.cause.code()
246246
{
247247
self.report_extra_impl_obligation(
248248
span,
@@ -263,7 +263,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
263263
}
264264
let trait_ref = trait_predicate.to_poly_trait_ref();
265265
let (post_message, pre_message, type_def) = self
266-
.get_parent_trait_ref(&obligation.cause.code)
266+
.get_parent_trait_ref(obligation.cause.code())
267267
.map(|(t, s)| {
268268
(
269269
format!(" in `{}`", t),
@@ -350,7 +350,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
350350
}
351351

352352
let explanation =
353-
if obligation.cause.code == ObligationCauseCode::MainFunctionType {
353+
if *obligation.cause.code() == ObligationCauseCode::MainFunctionType {
354354
"consider using `()`, or a `Result`".to_owned()
355355
} else {
356356
format!(
@@ -403,7 +403,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
403403
}
404404

405405
self.suggest_dereferences(&obligation, &mut err, &trait_ref, points_at_arg);
406-
self.suggest_borrow_on_unsized_slice(&obligation.cause.code, &mut err);
406+
self.suggest_borrow_on_unsized_slice(obligation.cause.code(), &mut err);
407407
self.suggest_fn_call(&obligation, &mut err, &trait_ref, points_at_arg);
408408
self.suggest_remove_reference(&obligation, &mut err, &trait_ref);
409409
self.suggest_semicolon_removal(&obligation, &mut err, span, &trait_ref);
@@ -1179,7 +1179,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
11791179
normalized_ty, data.ty
11801180
);
11811181

1182-
let is_normalized_ty_expected = match &obligation.cause.code {
1182+
let is_normalized_ty_expected = match obligation.cause.code() {
11831183
ObligationCauseCode::ItemObligation(_)
11841184
| ObligationCauseCode::BindingObligation(_, _)
11851185
| ObligationCauseCode::ObjectCastObligation(_) => false,
@@ -1435,7 +1435,10 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
14351435

14361436
debug!(
14371437
"maybe_report_ambiguity(predicate={:?}, obligation={:?} body_id={:?}, code={:?})",
1438-
predicate, obligation, body_id, obligation.cause.code,
1438+
predicate,
1439+
obligation,
1440+
body_id,
1441+
obligation.cause.code(),
14391442
);
14401443

14411444
// Ambiguity errors are often caused as fallout from earlier
@@ -1489,13 +1492,13 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
14891492
}
14901493
let mut err = self.need_type_info_err(body_id, span, self_ty, ErrorCode::E0283);
14911494
err.note(&format!("cannot satisfy `{}`", predicate));
1492-
if let ObligationCauseCode::ItemObligation(def_id) = obligation.cause.code {
1495+
if let ObligationCauseCode::ItemObligation(def_id) = *obligation.cause.code() {
14931496
self.suggest_fully_qualified_path(&mut err, def_id, span, trait_ref.def_id());
14941497
} else if let (
14951498
Ok(ref snippet),
14961499
ObligationCauseCode::BindingObligation(ref def_id, _),
14971500
) =
1498-
(self.tcx.sess.source_map().span_to_snippet(span), &obligation.cause.code)
1501+
(self.tcx.sess.source_map().span_to_snippet(span), obligation.cause.code())
14991502
{
15001503
let generics = self.tcx.generics_of(*def_id);
15011504
if generics.params.iter().any(|p| p.name.as_str() != "Self")
@@ -1685,7 +1688,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
16851688
self.note_obligation_cause_code(
16861689
err,
16871690
&obligation.predicate,
1688-
&obligation.cause.code,
1691+
obligation.cause.code(),
16891692
&mut vec![],
16901693
);
16911694
self.suggest_unsized_bound_if_applicable(err, obligation);
@@ -1698,7 +1701,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
16981701
obligation: &PredicateObligation<'tcx>,
16991702
) {
17001703
let (pred, item_def_id, span) =
1701-
match (obligation.predicate.kind(), &obligation.cause.code.peel_derives()) {
1704+
match (obligation.predicate.kind(), &obligation.cause.code().peel_derives()) {
17021705
(
17031706
ty::PredicateKind::Trait(pred, _),
17041707
ObligationCauseCode::BindingObligation(item_def_id, span),

‎src/librustc_trait_selection/traits/error_reporting/on_unimplemented.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
130130
self.describe_enclosure(obligation.cause.body_id).map(|s| s.to_owned()),
131131
));
132132

133-
match obligation.cause.code {
133+
match *obligation.cause.code() {
134134
ObligationCauseCode::BuiltinDerivedObligation(..)
135135
| ObligationCauseCode::ImplDerivedObligation(..)
136136
| ObligationCauseCode::DerivedObligation(..) => {}
@@ -142,7 +142,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
142142
}
143143

144144
if let ObligationCauseCode::ItemObligation(item)
145-
| ObligationCauseCode::BindingObligation(item, _) = obligation.cause.code
145+
| ObligationCauseCode::BindingObligation(item, _) = *obligation.cause.code()
146146
{
147147
// FIXME: maybe also have some way of handling methods
148148
// from other traits? That would require name resolution,
@@ -155,7 +155,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
155155
flags.push((sym::from_method, Some(method.to_string())));
156156
}
157157
}
158-
if let Some((t, _)) = self.get_parent_trait_ref(&obligation.cause.code) {
158+
if let Some((t, _)) = self.get_parent_trait_ref(obligation.cause.code()) {
159159
flags.push((sym::parent_trait, Some(t)));
160160
}
161161

‎src/librustc_trait_selection/traits/error_reporting/suggestions.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
476476
let param_env = obligation.param_env;
477477
let body_id = obligation.cause.body_id;
478478
let span = obligation.cause.span;
479-
let real_trait_ref = match &obligation.cause.code {
479+
let real_trait_ref = match obligation.cause.code() {
480480
ObligationCauseCode::ImplDerivedObligation(cause)
481481
| ObligationCauseCode::DerivedObligation(cause)
482482
| ObligationCauseCode::BuiltinDerivedObligation(cause) => &cause.parent_trait_ref,
@@ -691,7 +691,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
691691
let param_env = obligation.param_env;
692692
let trait_ref = trait_ref.skip_binder();
693693

694-
if let ObligationCauseCode::ImplDerivedObligation(obligation) = &obligation.cause.code {
694+
if let ObligationCauseCode::ImplDerivedObligation(obligation) = obligation.cause.code() {
695695
// Try to apply the original trait binding obligation by borrowing.
696696
let self_ty = trait_ref.self_ty();
697697
let found = self_ty.to_string();
@@ -951,7 +951,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
951951
obligation: &PredicateObligation<'tcx>,
952952
trait_ref: &ty::Binder<ty::TraitRef<'tcx>>,
953953
) -> bool {
954-
match obligation.cause.code.peel_derives() {
954+
match obligation.cause.code().peel_derives() {
955955
// Only suggest `impl Trait` if the return type is unsized because it is `dyn Trait`.
956956
ObligationCauseCode::SizedReturnType => {}
957957
_ => return false,
@@ -1145,7 +1145,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
11451145
err: &mut DiagnosticBuilder<'_>,
11461146
obligation: &PredicateObligation<'tcx>,
11471147
) {
1148-
match obligation.cause.code.peel_derives() {
1148+
match obligation.cause.code().peel_derives() {
11491149
ObligationCauseCode::SizedReturnType => {}
11501150
_ => return,
11511151
}
@@ -1339,7 +1339,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
13391339
};
13401340
let mut generator = None;
13411341
let mut outer_generator = None;
1342-
let mut next_code = Some(&obligation.cause.code);
1342+
let mut next_code = Some(obligation.cause.code());
13431343
while let Some(code) = next_code {
13441344
debug!("maybe_note_obligation_cause_for_async_await: code={:?}", code);
13451345
match code {

‎src/librustc_trait_selection/traits/fulfill.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ pub struct PendingPredicateObligation<'tcx> {
8484

8585
// `PendingPredicateObligation` is used a lot. Make sure it doesn't unintentionally get bigger.
8686
#[cfg(target_arch = "x86_64")]
87-
static_assert_size!(PendingPredicateObligation<'_>, 72);
87+
static_assert_size!(PendingPredicateObligation<'_>, 88);
8888

8989
impl<'a, 'tcx> FulfillmentContext<'tcx> {
9090
/// Creates a new fulfillment context.

‎src/librustc_trait_selection/traits/select/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2124,7 +2124,7 @@ impl<'tcx> TraitObligationExt<'tcx> for TraitObligation<'tcx> {
21242124
// by using -Z verbose or just a CLI argument.
21252125
let derived_cause = DerivedObligationCause {
21262126
parent_trait_ref: obligation.predicate.to_poly_trait_ref(),
2127-
parent_code: Rc::new(obligation.cause.code.clone()),
2127+
parent_code: Rc::new(obligation.cause.code().clone()),
21282128
};
21292129
let derived_code = variant(derived_cause);
21302130
ObligationCause::new(obligation.cause.span, obligation.cause.body_id, derived_code)

0 commit comments

Comments
 (0)