Implement explicit tail calls #112657

WaffleLapkin · 2023-06-15T14:48:26Z

r? @oli-obk
cc @scottmcm @phi-go @drmeepster

This is not complete, there are at least a couple of bugs (that I'm aware of, that is), however I'm starting to struggle because I've been looking at it too much, so a fresh look would be nice here.

I've tried to make commits as clean as I could, so hopefully rustfmt/clippy/libs/etc maintainers only need to look at a single commit that adds support there. (similarly all commits should pass tests at their point in time)

Note that this doesn't implement `become` correctly, there is still no early drop or tail calls involved. Moreother, it seems like MIR simply can not represent tail calls in its current form so I'll probably need to add new statement kinds or smh. to it in order to represent them...

rustbot · 2023-06-15T14:48:36Z

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/rustfmt

cc @rust-lang/rustfmt

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

This PR changes Stable MIR

cc @oli-obk, @celinval

Some changes occurred in rustc_ty_utils::consts.rs

cc @BoxyUwU

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

WaffleLapkin · 2023-06-15T14:56:56Z

compiler/rustc_mir_build/src/check_tail_calls.rs

+        // Erase regions since tail calls don't care about lifetimes
+        let callee_sig =
+            self.tcx.normalize_erasing_late_bound_regions(self.param_env, ty.fn_sig(self.tcx));


fn_sig here panics on the following snippet:

fn g() { become (&g)(); }

(or in general any snippet that involves &fn() and such)

WaffleLapkin · 2023-06-15T14:58:22Z

compiler/rustc_mir_build/src/check_tail_calls.rs

+            if caller_sig.output() != callee_sig.output() {
+                span_bug!(expr.span, "hir typeck should have checked the return type already");
+            }


This panics w/ opaque types, for example:

fn f() -> impl Sized { become g(); } fn g() {}

or

fn f() -> impl std::future::Future<Output = ()> { become g(); } async fn g() {}

both of those should probably work

JakobDegen

I sort of started to review some of this but this is proving pretty difficult. I think the PR is significantly too big. There is no one person with the expertise to review all of this. The history is clean right now, but that will not be true after this inevitably goes through half a dozen rounds of review with a handful of people.

I think we should be looking to split this up. Each individual PR should be small enough that it can be reviewed in its entirety by one person, in such a way that after that review is done we're confident that either the code is correct, or there are actual issues tracking critical bugs that may not be forgotten about.

JakobDegen · 2023-06-17T20:41:23Z

compiler/rustc_middle/src/mir/syntax.rs

+        /// These are owned by the callee, which is free to modify them.
+        /// This allows the memory occupied by "by-value" arguments to be
+        /// reused across function calls without duplicating the contents.


This doesn't make a whole lot of sense - all arguments are "by value" in Mir, operands are values after all (may edit this comment after I read the rest of this PR)

This comment is copied from TerminatorKind::Call::args

JakobDegen · 2023-06-17T20:43:35Z

compiler/rustc_mir_dataflow/src/framework/direction.rs

@@ -527,6 +527,10 @@ impl Direction for Forward {
                }
            }

+            TailCall { .. } => {
+                // FIXME(explicit_tail_calls): is there anything sensible to do here?


No; there are no successors

JakobDegen · 2023-06-17T20:44:40Z

compiler/rustc_mir_dataflow/src/framework/mod.rs

@@ -453,7 +453,7 @@ where
 /// building up a `GenKillSet` and then throwing it away.
 pub trait GenKill<T> {
    /// Inserts `elem` into the state vector.
-    fn gen(&mut self, elem: T);
+    fn gen(&mut self, der: T);


Accidentally left in? This doesn't do anything but the old name did make more sense to me

(i think it's a joke about fn gen(der))

(yeah; I accidentally commited it 💀 )

JakobDegen · 2023-06-17T20:57:44Z

compiler/rustc_mir_transform/src/generator.rs

@@ -1217,6 +1217,8 @@ fn can_unwind<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
            // These may unwind.
            TerminatorKind::Drop { .. }
            | TerminatorKind::Call { .. }
+            // FIXME(explicit_tail_calls): can tail calls unwind actually?..
+            | TerminatorKind::TailCall { .. }


Per the RFC, they aren't allowed in generators - I haven't yet checked if this PR implements this restriction (please unreachable! here)

RalfJung · 2023-06-18T18:06:33Z

Agreed with @JakobDegen. Can this be split up into smaller PRs that only touch certain parts of the compiler -- the frontend, MIR building, codegen, interpreter, all separate? IIRC this is what we did with inline const.

There's no reason to have the Miri/interpreter or codegen implementation in the same PR that implements the checks enforcing the become restrictions, or the new dropck/borrowck interactions.

WaffleLapkin · 2023-06-19T10:20:36Z

No problem, I'll split this up (should be easy, given that I can just cherry-pick necessary commits).

WaffleLapkin · 2023-06-19T11:02:02Z

I've opened a tracking issue (#112788) alongside the first two implementation PRs (parsing and llvm ffi) (see the tasklist in the issue).

…ompiler-errors implement checks for tail calls Quoting the [RFC draft](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md): > The argument to become is a function (or method) call, that exactly matches the function signature and calling convention of the callee. The intent is to ensure a matching ABI. Note that lifetimes may differ as long as they pass borrow checking, see [below](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md#return-type-coercion) for specifics on the return type. > Tail calling closures and tail calling from closures is not allowed. This is due to the high implementation effort, see below, this restriction can be lifted by a future RFC. > Invocations of operators were considered as valid targets but were rejected on grounds of being too error-prone. In any case, these can still be called as methods. > Tail calling [variadic functions](https://doc.rust-lang.org/beta/unstable-book/language-features/c-variadic.html) and tail calling from variadic functions is not allowed. As support for variadic function is stabilized on a per target level, support for tail-calls regarding variadic functions would need to follow a similar approach. To avoid this complexity and to minimize implementation effort for backends, this interaction is currently not allowed but support can be added with a future RFC. ----- The checks are implemented as a query, similarly to `check_unsafety`. The code is cherry-picked straight out of rust-lang#112657 which was written more than a year ago, so I expect we might need to change some things ^^"

Rollup merge of rust-lang#133607 - WaffleLapkin:tail-call-checks, r=compiler-errors implement checks for tail calls Quoting the [RFC draft](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md): > The argument to become is a function (or method) call, that exactly matches the function signature and calling convention of the callee. The intent is to ensure a matching ABI. Note that lifetimes may differ as long as they pass borrow checking, see [below](https://github.com/phi-go/rfcs/blob/guaranteed-tco/text/0000-explicit-tail-calls.md#return-type-coercion) for specifics on the return type. > Tail calling closures and tail calling from closures is not allowed. This is due to the high implementation effort, see below, this restriction can be lifted by a future RFC. > Invocations of operators were considered as valid targets but were rejected on grounds of being too error-prone. In any case, these can still be called as methods. > Tail calling [variadic functions](https://doc.rust-lang.org/beta/unstable-book/language-features/c-variadic.html) and tail calling from variadic functions is not allowed. As support for variadic function is stabilized on a per target level, support for tail-calls regarding variadic functions would need to follow a similar approach. To avoid this complexity and to minimize implementation effort for backends, this interaction is currently not allowed but support can be added with a future RFC. ----- The checks are implemented as a query, similarly to `check_unsafety`. The code is cherry-picked straight out of rust-lang#112657 which was written more than a year ago, so I expect we might need to change some things ^^"

WaffleLapkin and others added 13 commits June 15, 2023 11:57

Syntatically accept become expressions

56f216d

hir: Add Become expression kind

47ab1e5

Support tail calls in mir via TerminatorKind::TailCall

4005890

llvm ffi: Expose CallInst->setTailCallKind

dee279d

Support tail calls in rustc_codegen_{ssa,llvm}

0722a1e

handle drops for tail calls

c8b602a

Add more tail call tests

ced6c93

Implement necessary checks for tail calls

20c26dc

rustc_codegen_cranelift: bug on tail calls

0ace4e6

rustc_codegen_gcc: bug on tail calls

03b6764

Implement become expression formatting in rustfmt

cd7742c

Add support for tail calls in clippy

7cf2e9a

rustbot assigned oli-obk Jun 15, 2023

This comment has been minimized.

Sign in to view

WaffleLapkin commented Jun 15, 2023

View reviewed changes

WaffleLapkin added 2 commits June 15, 2023 15:26

Document become keyword

9afdbe6

Add out-of-{async,generators} tests

1d8efed

WaffleLapkin added 2 commits June 15, 2023 15:26

Add a test for mismatch in #[track_caller] wrt tail calls

65e7db6

Add some more tail call tests

965c7b6

rust-cloud-vms bot force-pushed the become_unuwuable branch from 3bdef3f to 965c7b6 Compare June 15, 2023 15:26

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Jun 15, 2023

JakobDegen reviewed Jun 17, 2023

View reviewed changes

WaffleLapkin closed this Jun 19, 2023

WaffleLapkin mentioned this pull request Nov 29, 2024

implement checks for tail calls #133607

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement explicit tail calls #112657

Implement explicit tail calls #112657

WaffleLapkin commented Jun 15, 2023 •

edited

Loading

rustbot commented Jun 15, 2023

This comment has been minimized.

WaffleLapkin Jun 15, 2023

WaffleLapkin Jun 15, 2023

JakobDegen left a comment

JakobDegen Jun 17, 2023

WaffleLapkin Jun 19, 2023

JakobDegen Jun 17, 2023

JakobDegen Jun 17, 2023

Noratrieb Jun 19, 2023

WaffleLapkin Jun 19, 2023

JakobDegen Jun 17, 2023

RalfJung commented Jun 18, 2023

WaffleLapkin commented Jun 19, 2023

WaffleLapkin commented Jun 19, 2023

Implement explicit tail calls #112657

Implement explicit tail calls #112657

Conversation

WaffleLapkin commented Jun 15, 2023 • edited Loading

rustbot commented Jun 15, 2023

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JakobDegen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RalfJung commented Jun 18, 2023

WaffleLapkin commented Jun 19, 2023

WaffleLapkin commented Jun 19, 2023

WaffleLapkin commented Jun 15, 2023 •

edited

Loading