Skip to content

Commit ec7b753

Browse files
committed
Auto merge of #85321 - cjgillot:mir-cycle, r=bjorn3
Use DefPathHash instead of HirId to break inlining cycles. The `DefPathHash` is stable across incremental compilation sessions, so provides a total order on `LocalDefId`. Using it instead of `HirId` ensures the MIR inliner has the same behaviour for incremental and non-incremental compilation. A downside is that the cycle tie break is not as predictable is with `HirId`.
2 parents 133859d + 297dde9 commit ec7b753

File tree

1 file changed

+12
-17
lines changed

1 file changed

+12
-17
lines changed

compiler/rustc_mir_transform/src/inline.rs

+12-17
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Inlining pass for MIR functions
22
33
use rustc_attr::InlineAttr;
4-
use rustc_hir as hir;
54
use rustc_index::bit_set::BitSet;
65
use rustc_index::vec::Idx;
76
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
@@ -88,7 +87,6 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
8887
tcx,
8988
param_env,
9089
codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
91-
hir_id,
9290
history: Vec::new(),
9391
changed: false,
9492
};
@@ -102,8 +100,6 @@ struct Inliner<'tcx> {
102100
param_env: ParamEnv<'tcx>,
103101
/// Caller codegen attributes.
104102
codegen_fn_attrs: &'tcx CodegenFnAttrs,
105-
/// Caller HirID.
106-
hir_id: hir::HirId,
107103
/// Stack of inlined Instances.
108104
history: Vec<ty::Instance<'tcx>>,
109105
/// Indicates that the caller body has been modified.
@@ -179,7 +175,9 @@ impl<'tcx> Inliner<'tcx> {
179175
caller_body: &Body<'tcx>,
180176
callee: &Instance<'tcx>,
181177
) -> Result<(), &'static str> {
182-
if callee.def_id() == caller_body.source.def_id() {
178+
let caller_def_id = caller_body.source.def_id();
179+
let callee_def_id = callee.def_id();
180+
if callee_def_id == caller_def_id {
183181
return Err("self-recursion");
184182
}
185183

@@ -188,7 +186,7 @@ impl<'tcx> Inliner<'tcx> {
188186
// If there is no MIR available (either because it was not in metadata or
189187
// because it has no MIR because it's an extern function), then the inliner
190188
// won't cause cycles on this.
191-
if !self.tcx.is_mir_available(callee.def_id()) {
189+
if !self.tcx.is_mir_available(callee_def_id) {
192190
return Err("item MIR unavailable");
193191
}
194192
}
@@ -208,29 +206,26 @@ impl<'tcx> Inliner<'tcx> {
208206
| InstanceDef::CloneShim(..) => return Ok(()),
209207
}
210208

211-
if self.tcx.is_constructor(callee.def_id()) {
209+
if self.tcx.is_constructor(callee_def_id) {
212210
trace!("constructors always have MIR");
213211
// Constructor functions cannot cause a query cycle.
214212
return Ok(());
215213
}
216214

217-
if let Some(callee_def_id) = callee.def_id().as_local() {
218-
let callee_hir_id = self.tcx.hir().local_def_id_to_hir_id(callee_def_id);
215+
if callee_def_id.is_local() {
219216
// Avoid a cycle here by only using `instance_mir` only if we have
220-
// a lower `HirId` than the callee. This ensures that the callee will
221-
// not inline us. This trick only works without incremental compilation.
222-
// So don't do it if that is enabled.
223-
if !self.tcx.dep_graph.is_fully_enabled() && self.hir_id.index() < callee_hir_id.index()
217+
// a lower `DefPathHash` than the callee. This ensures that the callee will
218+
// not inline us. This trick even works with incremental compilation,
219+
// since `DefPathHash` is stable.
220+
if self.tcx.def_path_hash(caller_def_id).local_hash()
221+
< self.tcx.def_path_hash(callee_def_id).local_hash()
224222
{
225223
return Ok(());
226224
}
227225

228226
// If we know for sure that the function we're calling will itself try to
229227
// call us, then we avoid inlining that function.
230-
if self
231-
.tcx
232-
.mir_callgraph_reachable((*callee, caller_body.source.def_id().expect_local()))
233-
{
228+
if self.tcx.mir_callgraph_reachable((*callee, caller_def_id.expect_local())) {
234229
return Err("caller might be reachable from callee (query cycle avoidance)");
235230
}
236231

0 commit comments

Comments
 (0)