r? @lcnr

rustbot has assigned @lcnr.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

I don't think this is the right approach. Frankly, using caller_ty seems a bit unnecessary throughout check_tail_calls, though that's preexisting.

I think it's better if we refactor check_tail_calls to no longer rely at all on caller_ty and instead just have a body def id which corresponds to the item that has the THIR. We shouldn't need substs either -- those are always identity.

Then we shouldn't need to use Instance::expect_resolve at all here.

Thank you for your feedback! I addressed it. There's a test failing in LLVM codegen, but this may be a pre-existing issue.

The job aarch64-gnu-llvm-19-1 failed! Check out the build log: (web) (plain enhanced) (plain)

---- [ui] tests/ui/explicit-tail-calls/default-trait-method.rs stdout ----

error: test compilation failed although it shouldn't!
status: exit status: 101
command: env -u RUSTC_LOG_COLOR RUSTC_ICE="0" RUST_BACKTRACE="short" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/tests/ui/explicit-tail-calls/default-trait-method.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=/cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=/checkout/vendor" "--sysroot" "/checkout/obj/build/aarch64-unknown-linux-gnu/stage2" "--target=aarch64-unknown-linux-gnu" "--check-cfg" "cfg(test,FALSE)" "-O" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/aarch64-unknown-linux-gnu/test/ui/explicit-tail-calls/default-trait-method/a" "-A" "internal_features" "-A" "unused_parens" "-A" "unused_braces" "-Crpath" "-Cdebuginfo=0" "-Lnative=/checkout/obj/build/aarch64-unknown-linux-gnu/native/rust-test-helpers"
stdout: none
--- stderr -------------------------------
cannot guarantee tail call due to mismatched parameter counts
  %2 = musttail call noundef i64 @"_ZN81_$LT$default_trait_method..OtherStruct$u20$as$u20$default_trait_method..Trait$GT$3bar17h8ff568ee08306d70E"(ptr noalias noundef nonnull readonly align 1 %0, ptr noalias noundef readonly align 8 dereferenceable(24) @2)
rustc-LLVM ERROR: Broken module found, compilation aborted!
------------------------------------------



failures:

Unfortunately, that's a legitimate bug.

I think the problem with #[track_caller] is deeper than this -- specifically, I think we actually need to disallow calling #[track_caller] functions altogether (which I think we can only check for during monomorphization), or at least we need to be adjusting such calls to a "reify shim" when monomorphizing those tail calls, though I think that ends up leading to us pushing a stack frame which seems not good.

I'm gonna mark this as blocked on #145321 (edit: actually #144755)

@rustbot blocked

As a side-note, please make sure your commits are squashed. It's not a requirement that you have 1 commit per PR, but this PR definitely shouldn't need 4 commits for a single change like this :)

r? compiler-errors

compiler-errors is not on the review rotation at the moment.
They may take a while to respond.

I think the problem with #[track_caller] is deeper than this -- specifically, I think we actually need to disallow calling #[track_caller] functions altogether (which I think we can only check for during monomorphization), or at least we need to be adjusting such calls to a "reify shim" when monomorphizing those tail calls, though I think that ends up leading to us pushing a stack frame which seems not good.

I'm gonna mark this as blocked on #145321 (edit: actually #144755)

@compiler-errors, thanks for elaborating on this!

So, as I gather from #145321 and #144755, if this is a pre-existing issue, would it be okay for us to move forward with this PR if we change the relevant test to be check-pass rather than run-pass, and let the codegen issue be independently covered by #145321?

As a side-note, please make sure your commits are squashed. It's not a requirement that you have 1 commit per PR, but this PR definitely shouldn't need 4 commits for a single change like this :)

Sure. I tend to make a commit per a logical chunk of work, and then in the course of PR’s review, only add new commits to make it clear what new changes have been made. But, of course, I’ll adjust and squash this, if you can confirm that converting the test to check-pass would be a sensible change.

☔ The latest upstream changes (presumably #145423) made this pull request unmergeable. Please resolve the merge conflicts.