Rewrite the UnconditionalRecursion lint to use MIR

[wesleywiser]

Part of #51002

r? @eddyb

[eddyb]

Do you need this describe_def check? I'd suspect that if you have issues with the substs, you'd rather normalize them somehow. Polymorphic recursion wouldn't compile anyway.

[wesleywiser]

Just doing the substs check missed a few cases which the old lint caught. For example:

fn test<T: Copy>() {
  test::<usize>();
}

The old lint caught this. With the additional check for "are we in a trait method", all the existing tests pass for the new lint.

[eddyb]

Ah, that is unfortunate, since it's not polymorphic recursion, and doesn't involve traits. I don't like describe_def though - can you use some functionality on tcx to check whether the function is a trait-associated item? Inherent methods, or methods in impls, need to behave like ordinary functions, it's only trait impls that care about dispatch.

Furthermore, only the part of the substs that corresponds to the trait, is relevant here, e.g. if the method has generics, they should probably be ignored.

[wesleywiser]

Hey @eddyb, I think I resolved most of your feedback except for this bit:

Furthermore, only the part of the substs that corresponds to the trait, is relevant here, e.g. if the method has generics, they should probably be ignored.

I'm not really sure how to go about doing that. Do you have any pointers?

[eddyb]

@wesleywiser You can slice both substs with ..tcx.generics_of(trait_def_id).count().

[wesleywiser]

Thanks @eddyb! I think all of your feedback is resolved now.

[wesleywiser]

Rebased and ready for review.

[TimNN]

Ping from triage @eddyb: This PR requires your review.

[oli-obk]

@bors r+

I have nothing more to add, lgtm

[bors]

📌 Commit f81d1dd has been approved by oli-obk

[wesleywiser]

Thanks @oli-obk!

[bors]

⌛ Testing commit f81d1dd with merge a4d1ac8e4337a605ee45ae8e706a550ceaa509ae...

[bors]

💔 Test failed - status-travis

[rust-highfive]

The job x86_64-gnu-distcheck of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

[00:05:36] Caused by:
[00:05:36]   failed to download package from registry
[00:05:36] 
[00:05:36] Caused by:
[00:05:36]   failed to get 200 response from `https://crates.io/api/v1/crates/openssl-src/111.0.1+1.1.1/download`, got 403
[00:05:36] 
[00:05:36] 
[00:05:36] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "vendor"
[00:05:36] 
[00:05:36] 
[00:05:36] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test distcheck
[00:05:36] Build completed unsuccessfully in 0:02:08
---
travis_time:end:044c5b6a:start=1540499353942303725,finish=1540499353951328642,duration=9024917
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:19d56c28
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:05e00bc0
travis_time:start:05e00bc0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:07ccdffe
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

[wesleywiser]

Looks like a spurious crates.io download failure?

[wesleywiser]

@bors retry

[bors]

⌛ Testing commit f81d1dd with merge 4bd4e41...

[oli-obk]

yea this has been going around, I think most of the queue has been hit by it

[wesleywiser]

Should we open a tracking issue for that?

[oli-obk]

nah, infra is already ahead of it on discord

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing 4bd4e41 to master...