Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

missing async spill when returning a function call of async fn with error union return value #3190

Closed
andrewrk opened this issue Sep 7, 2019 · 6 comments
Labels
bug Observed behavior contradicts documented or intended behavior
Milestone

Comments

@andrewrk
Copy link
Member

andrewrk commented Sep 7, 2019

const std = @import("std");
const expect = std.testing.expect;

var global_frame: anyframe = undefined;

test "try async call" {
    var f = async atest();
    resume global_frame;
    // TODO need https://github.com/ziglang/zig/issues/3157 for this next line
    //expect((noasync await f) == 1234);
}

fn atest() !i32 {
    return fallible1();
}

fn fallible1() anyerror!i32 {
    suspend {
        global_frame = @frame();
    }
    return 1234;
}

Expected to pass. Actual output:

Instruction does not dominate all uses!
  %20 = load { i32, i16 }*, { i32, i16 }** %3, align 8, !dbg !1039
  %31 = bitcast { i32, i16 }* %20 to i8*, !dbg !1039
LLVM ERROR: Broken module found, compilation aborted!

Related: #3157 (for the convenience of the test case)

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Sep 7, 2019
@andrewrk andrewrk added this to the 0.5.0 milestone Sep 7, 2019
@Vexu
Copy link
Member

Vexu commented Nov 7, 2019

This still seems to be happening.

test "correctly spill when returning the error union result of another async fn" {
    const S = struct {
        const Self = @This();
        term: ?(SpawnError!Term),

        pub const SpawnError = error{OutOfMemory};

        pub const Term = union(enum) {
            Exited: u32,
            Signal: u32,
            Stopped: u32,
            Unknown: u32,
        };

        var global_frame: anyframe = undefined;

        fn doTheTest(self: *Self) void {
            atest(self);
            if ((self.term.? catch unreachable) != .Exited) unreachable;
        }

        fn atest(self: *Self) void {
            self.term = fallible1(self);
        }

        fn fallible1(self: *Self) SpawnError!Term {
            suspend {
                global_frame = @frame();
            }
            return Term{.Exited = 0};
        }
    };
    var s = S {.term = undefined};
    _ = async s.doTheTest();
    resume S.global_frame;
}

Results in

broken LLVM module found: Instruction does not dominate all uses!
  %26 = load { %Term, i16 }*, { %Term, i16 }** %5, align 8, !dbg !1858
  %37 = bitcast { %Term, i16 }* %26 to i8*, !dbg !1858

@andrewrk andrewrk reopened this Nov 7, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Nov 7, 2019
Vexu added a commit to Vexu/zig that referenced this issue Nov 26, 2019
Vexu added a commit to Vexu/zig that referenced this issue Nov 26, 2019
@andrewrk
Copy link
Member Author

andrewrk commented Feb 8, 2020

Fixed by #4404

@andrewrk
Copy link
Member Author

Landed in 014f66e

@czipperz
Copy link
Contributor

@andrewrk Should you remove this comment

// TODO https://github.com/ziglang/zig/issues/3190
?

@czipperz
Copy link
Contributor

Unrelated but this looks like dead code

errdefer allocator.destroy(child);

@andrewrk
Copy link
Member Author

@andrewrk Should you remove this comment

Yes and collapse those 2 lines back into 1.

Unrelated but this looks like dead code

I think dead code in errdefers are ok. They document resource cleanup that would need to happen if you added additional code after that line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

3 participants