…gillot

Be a bit more careful around exotic cycles in in the inliner

Copied from the comment here: rust-lang#143700 (comment)

---

```rust
#![feature(fn_traits)]

#[inline]
pub fn a() {
    FnOnce::call_once(a, ());
    FnOnce::call_once(b, ());
}

#[inline]
pub fn b() {
    FnOnce::call_once(b, ());
    FnOnce::call_once(a, ());
}
```

This should demonstrate the issue. For ease of discussion, I'm gonna call the two fn-def types `{a}` and `{b}`.

When collecting the cyclic local callees in `mir_callgraph_cyclic` for `a`, we first check the first call terminator in `a`. We end up calling process on `<{a} as FnOnce>::call_once`, which ends up visiting `a`'s instance again. This is cyclical. However, we don't end up marking `FnOnce::call_once` as a cyclical def id because it's a foreign item. That's fine.

When visiting the second call terminator in `a`, which is `<{b} as FnOnce>::call_once`, we end up recursing into `b`. We check the first terminator, which is `<{b} as FnOnce>::call_once`, but although that is its own mini cycle, it doesn't consider itself a cycle for the purpose of this query because it doesn't involve the *root*. However, when we visit the *second* terminator in `b`, which is `<{a} as FnOnce>::call_once`, we end up **erroneously** *not* considering that call to be cyclical since we've already inserted it into our set of seen instances, and as a consequence we don't recurse into it. This means that we never collect `b` as recursive.

Do this in the flipped case too, and we end up having two functions which mututally do not consider each other to be recursive participants. This leads to a query cycle.

---

I ended up also renaming some variables so I could more clearly understand their responsibilities in this code. Let me know if the renames are not welcome.

Fixes rust-lang#143700
r? `@cjgillot`

Rollup of 9 pull requests

Successful merges:

 - #143213 (de-duplicate condition scoping logic between AST→HIR lowering and `ScopeTree` construction)
 - #143461 (make `cfg_select` a builtin macro)
 - #143519 (Check assoc consts and tys later like assoc fns)
 - #143554 (slice: Mark `rotate_left`, `rotate_right` unstably const)
 - #143704 (Be a bit more careful around exotic cycles in in the inliner)
 - #143774 (constify `From` and `Into`)
 - #143786 (Fix fallback for CI_JOB_NAME)
 - #143796 (Fix ICE for parsed attributes with longer path not handled by CheckAttribute)
 - #143798 (Remove format short command trait)

r? `@ghost`
`@rustbot` modify labels: rollup

…gillot

Be a bit more careful around exotic cycles in in the inliner

Copied from the comment here: rust-lang#143700 (comment)

---

```rust
#![feature(fn_traits)]

#[inline]
pub fn a() {
    FnOnce::call_once(a, ());
    FnOnce::call_once(b, ());
}

#[inline]
pub fn b() {
    FnOnce::call_once(b, ());
    FnOnce::call_once(a, ());
}
```

This should demonstrate the issue. For ease of discussion, I'm gonna call the two fn-def types `{a}` and `{b}`.

When collecting the cyclic local callees in `mir_callgraph_cyclic` for `a`, we first check the first call terminator in `a`. We end up calling process on `<{a} as FnOnce>::call_once`, which ends up visiting `a`'s instance again. This is cyclical. However, we don't end up marking `FnOnce::call_once` as a cyclical def id because it's a foreign item. That's fine.

When visiting the second call terminator in `a`, which is `<{b} as FnOnce>::call_once`, we end up recursing into `b`. We check the first terminator, which is `<{b} as FnOnce>::call_once`, but although that is its own mini cycle, it doesn't consider itself a cycle for the purpose of this query because it doesn't involve the *root*. However, when we visit the *second* terminator in `b`, which is `<{a} as FnOnce>::call_once`, we end up **erroneously** *not* considering that call to be cyclical since we've already inserted it into our set of seen instances, and as a consequence we don't recurse into it. This means that we never collect `b` as recursive.

Do this in the flipped case too, and we end up having two functions which mututally do not consider each other to be recursive participants. This leads to a query cycle.

---

I ended up also renaming some variables so I could more clearly understand their responsibilities in this code. Let me know if the renames are not welcome.

Fixes rust-lang#143700
r? ``@cjgillot``

Rollup of 14 pull requests

Successful merges:

 - #143301 (`tests/ui`: A New Order [26/N])
 - #143461 (make `cfg_select` a builtin macro)
 - #143519 (Check assoc consts and tys later like assoc fns)
 - #143554 (slice: Mark `rotate_left`, `rotate_right` unstably const)
 - #143634 (interpret/allocation: expose init + write_wildcards on a range)
 - #143679 (Preserve the .debug_gdb_scripts section)
 - #143685 (Resolve: merge `source_bindings` and `target_bindings` into `bindings`)
 - #143704 (Be a bit more careful around exotic cycles in in the inliner)
 - #143734 (Refactor resolve resolution bindings)
 - #143774 (constify `From` and `Into`)
 - #143785 (Add --compile-time-deps argument for x check)
 - #143786 (Fix fallback for CI_JOB_NAME)
 - #143825 (clippy: fix test filtering when TESTNAME is empty)
 - #143826 (Fix command trace)

r? `@ghost`
`@rustbot` modify labels: rollup

…gillot

Be a bit more careful around exotic cycles in in the inliner

Copied from the comment here: rust-lang#143700 (comment)

---

```rust
#![feature(fn_traits)]

#[inline]
pub fn a() {
    FnOnce::call_once(a, ());
    FnOnce::call_once(b, ());
}

#[inline]
pub fn b() {
    FnOnce::call_once(b, ());
    FnOnce::call_once(a, ());
}
```

This should demonstrate the issue. For ease of discussion, I'm gonna call the two fn-def types `{a}` and `{b}`.

When collecting the cyclic local callees in `mir_callgraph_cyclic` for `a`, we first check the first call terminator in `a`. We end up calling process on `<{a} as FnOnce>::call_once`, which ends up visiting `a`'s instance again. This is cyclical. However, we don't end up marking `FnOnce::call_once` as a cyclical def id because it's a foreign item. That's fine.

When visiting the second call terminator in `a`, which is `<{b} as FnOnce>::call_once`, we end up recursing into `b`. We check the first terminator, which is `<{b} as FnOnce>::call_once`, but although that is its own mini cycle, it doesn't consider itself a cycle for the purpose of this query because it doesn't involve the *root*. However, when we visit the *second* terminator in `b`, which is `<{a} as FnOnce>::call_once`, we end up **erroneously** *not* considering that call to be cyclical since we've already inserted it into our set of seen instances, and as a consequence we don't recurse into it. This means that we never collect `b` as recursive.

Do this in the flipped case too, and we end up having two functions which mututally do not consider each other to be recursive participants. This leads to a query cycle.

---

I ended up also renaming some variables so I could more clearly understand their responsibilities in this code. Let me know if the renames are not welcome.

Fixes rust-lang#143700
r? `@cjgillot`

Rollup of 12 pull requests

Successful merges:

 - #143280 (Remove duplicate error about raw underscore lifetime)
 - #143649 (Add test for `default_field_values` and `const_default`)
 - #143699 (Make `AsyncDrop` check that it's being implemented on a local ADT)
 - #143704 (Be a bit more careful around exotic cycles in in the inliner)
 - #143908 (`tests/ui`: A New Order [0/28] )
 - #143909 (docs(alloc::fmt): Make type optional, instead of matching empty string)
 - #143925 (Make slice comparisons const)
 - #143997 (Use $crate in macros for rustc_public (aka stable_mir))
 - #144013 (resolve: Make disambiguators for underscore bindings module-local)
 - #144029 (Fix wrong messages from methods with the same name from different traits)
 - #144063 (Add myself to the `infra-ci` reviewer group and adjust some infra auto-labels)
 - #144069 (ci: use windows 22 for all free runners)

r? `@ghost`
`@rustbot` modify labels: rollup

Rollup of 10 pull requests

Successful merges:

 - #141076 (fix Zip unsoundness (again))
 - #142444 (adding run-make test to autodiff)
 - #143704 (Be a bit more careful around exotic cycles in in the inliner)
 - #144073 (Don't test panic=unwind in panic_main.rs on Fuchsia)
 - #144083 (miri sleep tests: increase slack)
 - #144092 (bootstrap: Detect musl hosts)
 - #144098 (Do not lint private-in-public for RPITIT)
 - #144103 (Rename `emit_unless` to `emit_unless_delay`)
 - #144108 (Ignore tests/run-make/link-eh-frame-terminator/rmake.rs when cross-compiling)
 - #144115 (fix outdated comment)

r? `@ghost`
`@rustbot` modify labels: rollup

Rollup merge of #143704 - compiler-errors:cycle-exotic, r=cjgillot

Be a bit more careful around exotic cycles in in the inliner

Copied from the comment here: #143700 (comment)

---

```rust
#![feature(fn_traits)]

#[inline]
pub fn a() {
    FnOnce::call_once(a, ());
    FnOnce::call_once(b, ());
}

#[inline]
pub fn b() {
    FnOnce::call_once(b, ());
    FnOnce::call_once(a, ());
}
```

This should demonstrate the issue. For ease of discussion, I'm gonna call the two fn-def types `{a}` and `{b}`.

When collecting the cyclic local callees in `mir_callgraph_cyclic` for `a`, we first check the first call terminator in `a`. We end up calling process on `<{a} as FnOnce>::call_once`, which ends up visiting `a`'s instance again. This is cyclical. However, we don't end up marking `FnOnce::call_once` as a cyclical def id because it's a foreign item. That's fine.

When visiting the second call terminator in `a`, which is `<{b} as FnOnce>::call_once`, we end up recursing into `b`. We check the first terminator, which is `<{b} as FnOnce>::call_once`, but although that is its own mini cycle, it doesn't consider itself a cycle for the purpose of this query because it doesn't involve the *root*. However, when we visit the *second* terminator in `b`, which is `<{a} as FnOnce>::call_once`, we end up **erroneously** *not* considering that call to be cyclical since we've already inserted it into our set of seen instances, and as a consequence we don't recurse into it. This means that we never collect `b` as recursive.

Do this in the flipped case too, and we end up having two functions which mututally do not consider each other to be recursive participants. This leads to a query cycle.

---

I ended up also renaming some variables so I could more clearly understand their responsibilities in this code. Let me know if the renames are not welcome.

Fixes #143700
r? `@cjgillot`

…gillot

Be a bit more careful around exotic cycles in in the inliner

Copied from the comment here: rust-lang#143700 (comment)

---

```rust
#![feature(fn_traits)]

#[inline]
pub fn a() {
    FnOnce::call_once(a, ());
    FnOnce::call_once(b, ());
}

#[inline]
pub fn b() {
    FnOnce::call_once(b, ());
    FnOnce::call_once(a, ());
}
```

This should demonstrate the issue. For ease of discussion, I'm gonna call the two fn-def types `{a}` and `{b}`.

When collecting the cyclic local callees in `mir_callgraph_cyclic` for `a`, we first check the first call terminator in `a`. We end up calling process on `<{a} as FnOnce>::call_once`, which ends up visiting `a`'s instance again. This is cyclical. However, we don't end up marking `FnOnce::call_once` as a cyclical def id because it's a foreign item. That's fine.

When visiting the second call terminator in `a`, which is `<{b} as FnOnce>::call_once`, we end up recursing into `b`. We check the first terminator, which is `<{b} as FnOnce>::call_once`, but although that is its own mini cycle, it doesn't consider itself a cycle for the purpose of this query because it doesn't involve the *root*. However, when we visit the *second* terminator in `b`, which is `<{a} as FnOnce>::call_once`, we end up **erroneously** *not* considering that call to be cyclical since we've already inserted it into our set of seen instances, and as a consequence we don't recurse into it. This means that we never collect `b` as recursive.

Do this in the flipped case too, and we end up having two functions which mututally do not consider each other to be recursive participants. This leads to a query cycle.

---

I ended up also renaming some variables so I could more clearly understand their responsibilities in this code. Let me know if the renames are not welcome.

Fixes rust-lang#143700
r? `@cjgillot`