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

Passing continuation to function crashes compiler #7

Open
michael-kenzel opened this issue Sep 8, 2021 · 9 comments
Open

Passing continuation to function crashes compiler #7

michael-kenzel opened this issue Sep 8, 2021 · 9 comments
Labels
bug Something isn't working

Comments

@michael-kenzel
Copy link
Contributor

michael-kenzel commented Sep 8, 2021

The following code will reproduce the issue:

fn test() {
	let blub = @|cont: fn()->()| {};

	while (true) {
		blub(continue)
	}
}

Compiling the above code results in:

terminate called after throwing an instance of 'std::bad_array_new_length'
  what():  std::bad_array_new_length

I assume this may be caused by the incorrect return type in the function type of the parameter the continuation is passed to.

One workaround seems to be wrapping the continuation in a lambda:

fn test() {
	let blub = @|cont: fn()->()| {};

	while (true) {
		blub(@||continue())
	}
}
@michael-kenzel michael-kenzel added the bug Something isn't working label Sep 8, 2021
@Hugobros3
Copy link
Contributor

So this is actually type-sound, as the type of continue is fn() -> !, with ! being the bottom (or more specifically, no-return) type. Because continue never returns, it also can be validly placed in contexts excepting some return value, as it will never return a wrong type (since it never returns!).

We run into an issue however when trying to emit Thorin code for this, because thorin encodes returns with another parameter, so the type of the cont (the param of blub) is fn(fn() -> !) -> !, but we're giving it a fn() -> !, and so the generated thorin code doesn't type.

fn blub (cont: fn(fn() -> !) -> !, ret: fn() -> !) = ret()

fn while_loop_continue() = while_loop_head()

fn test_while_...() = blub(while_loop_continue)

The solution ? Well Artic actually has some code already to deal with this edge case, but it isn't used in all contexts it seems, in particular when auto-casting it doesn't get called, so this is hopefully a simple refactor to fix.

@Hugobros3
Copy link
Contributor

Hugobros3 commented Sep 14, 2021

Wrote a fix (6cf219d), the output now looks sensible:

test_6: fn[mem, fn[mem]] = {
    test_6: fn[mem, fn[mem]] = while_head_15(test_7[test_6])
}

while_head_15: fn[mem] = {
    while_head_15: fn[mem] = branch_true_25()
}

branch_true_25: fn[] = {
    branch_true_25: fn[] = while_body_23(while_head_16[while_head_15])
}

while_body_23: fn[mem] = {
    while_body_23: fn[mem] = blub_10(while_body_24[while_body_23], _27, cont_30)
}

blub_10: fn[mem, fn[mem, fn[mem]], fn[mem]] = {
    blub_10: fn[mem, fn[mem, fn[mem]], fn[mem]] = ret_13[blub_10](_11[blub_10])
}

_27: fn[mem, fn[mem]] = {
    _27: fn[mem, fn[mem]] = while_continue_19(_28[_27])
}

cont_30: fn[mem] = {
    cont_30: fn[mem] = while_head_15(cont_31[cont_30])
}

while_continue_19: fn[mem] = {
    while_continue_19: fn[mem] = while_head_15(while_continue_20[while_continue_19])
}

@madmann91
Copy link
Contributor

Is this fixed? Can we close the issue?

@Hugobros3
Copy link
Contributor

I think so, I did write a fix, sadly the specifics are fuzzy to me. Since there was no activity afterwards, I guess that did it!

Another issue can be opened if a similar problem arises.

@michael-kenzel
Copy link
Contributor Author

I'm afraid the problem does not appear to be resolved completely as I've run into it again.

The following example

fn ohnoes(throw: fn(&[u8]) -> ()) {
	throw("");
}

#[export]
fn test() -> () {
	let throw = return;
	let handle_failure = @|msg: &[u8]| {
		throw()
	};

	ohnoes(handle_failure);
}

results in

src/emit.cpp:776: void artic::Emitter::bind(const artic::ast::IdPtrn&, const thorin::Def*): Assertion `id_ptrn.type->convert(*this) == value->type()' failed.

And this slight variation of the example:

fn ohnoes(throw: fn() -> ()) {
	throw();
}

#[export]
fn test() -> () {
	let throw = return;
	let handle_failure = @|| {
		throw()
	};

	ohnoes(handle_failure);
}

triggers

terminate called after throwing an instance of 'std::bad_array_new_length'
  what():  std::bad_array_new_length

Hugobros3 added a commit that referenced this issue Oct 14, 2022
@Hugobros3
Copy link
Contributor

Hugobros3 commented Oct 14, 2022

I just debugged this, basically handle_failure looks like a basic block to Artic because it does not return (it calls throw that does not return either), but it is later assumed to be a function and that causes an off-by-one error in tuple_from_params. I have a basic fix that I just sent, but eventually I plan to add a proper distinction between functions and basic blocks that doesn't just look at whether they return something or not. (I want to support functions that don't return as first-class functions)

@michael-kenzel
Copy link
Contributor Author

I'm afraid this now seems to break the following code that's part of the runtime (reduced sample):

#[import(cc = "thorin")] fn opencl(_dev: i32, _grid: (i32, i32, i32), _block: (i32, i32, i32), _body: fn() -> ()) -> ();

struct WorkItem {}

struct Accelerator {
    exec : fn(fn(WorkItem) -> ()) -> fn((i32, i32, i32), (i32, i32, i32)) -> (), // fn(grid, block)->()
}

fn @opencl_accelerator(dev: i32) = Accelerator {
    exec = @|body| |grid, block| {
        opencl(dev, grid, block, || @body(WorkItem {}))
    }
};

produces

/src/emit.cpp:776: void artic::Emitter::bind(const artic::ast::IdPtrn&, const thorin::Def*): Assertion `id_ptrn.type->convert(*this) == value->type()' failed.

@PearCoding
Copy link
Contributor

I also get an Access Violation with the new changes while compiling code. I would suggest taking the last commit back until more information is available?

@Hugobros3
Copy link
Contributor

Sorry, that's what I get for pushing a "trivial" fix, on a Friday no less! I rolled back the patch and pushed again.

I will open another issue on Monday to keep track of the wider problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants