mir: adjust conditional in recursion limit check

[davidtwco]

Fixes #67552.

This PR adjusts the condition used in the recursion limit check of
the monomorphization collector, from > to >=.

In #67552, the test case had infinite indirect recursion, repeating a
handful of functions (from the perspective of the monomorphization
collector): rec -> identity -> Iterator::count -> Iterator::fold
-> Iterator::next -> rec.

During this process, resolve_associated_item was invoked for
Iterator::fold (during the construction of an Instance), and
ICE'd due to substitutions needing inference. However, previous
iterations of this recursion would have called this function for
Iterator::fold - and did! - and succeeded in doing so (trivially
checkable from debug logging, () is present where _ is in the substs
of the failing execution).

The expected outcome of this test case would be a recursion limit error
(which is present when the identity fn indirection is removed), and
the recursion depth of rec is increasing (other functions finish
collecting their neighbours and thus have their recursion depths reset).

When the ICE occurs, the recursion depth of rec is 256 (which matches
the recursion limit), which suggests perhaps that a different part of
the compiler is using a >= comparison and returning a different result
on this recursion rather than what it returned in every previous
recursion, thus stopping the monomorphization collector from reporting
an error on the next recursion, where recursion_depth_of_rec > 256
would have been true.

With grep and some educated guesses, we can determine that
the recursion limit check at line 818 in
src/librustc_trait_selection/traits/project.rs is the other check that
is using a different comparison. Modifying either comparison to be > or
>= respectively will fix the error, but changing the monomorphization
collector produces the nicer error.

[rust-highfive]

r? @varkor

(rust_highfive has picked a reviewer for you, use r? to override)

[varkor]

It seems this inconsistency is found in more places throughout the compiler, e.g. in these two locations
https://github.com/rust-lang/rust/blob/a59264b01247836c70e24217e0d346b868387525/src/librustc_ty/needs_drop.rs#L70-L72
https://github.com/rust-lang/rust/blob/a59264b01247836c70e24217e0d346b868387525/src/librustc_mir/interpret/eval_context.rs#L635-L637
These should probably be fixed at the same time. Also, this behaviour should be documented, e.g. in https://doc.rust-lang.org/reference/attributes/limits.html (though for now, we could just open an issue). It would be good to make sure the other limit attributes behave similarly.

Do you think you could audit these limits and add a comment somewhere appropriate mentioning this?

[Mark-Simulacrum]

It seems reasonable to add a struct somewhere with check and increment methods that would store recursion limits and be used throughout the compiler, to hopefully prevent this from happening again.

[davidtwco]

I've added a commit with a Limit type, based on @Mark-Simulacrum's suggestion. I chose to make the comparison value <= limit, as while this did result in a larger diff, it behaved more like I would expect - in the macro expansion recursion limit tests, a limit of four would show four "in expansion of" lines - which matched my expectations. I'll submit an issue to the reference shortly. I've submitted rust-lang/reference#820.

[varkor]

This looks good to me. r=me with the name change, or similar.

[davidtwco]

@bors r=varkor

[bors]

📌 Commit a54ed87 has been approved by varkor