Skip to content

Commit

Permalink
Disallow return at the end of a with block (#4467)
Browse files Browse the repository at this point in the history
It generates incorrect LLVM IR in some scenarios and can also lead
to compiler crashes.

Fixes #4464
  • Loading branch information
SeanTAllen authored Oct 29, 2023
1 parent 7705105 commit b9f3b6b
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 21 deletions.
40 changes: 40 additions & 0 deletions .release-notes/4464.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
## Disallow `return` at the end of a `with` block

When we reimplemented `with` blocks in 2022, we allowed the usage of `return` at the end of the block. Recent analysis of the generated LLVM flagged incorrect LLVM IR that was generated from such code.

Upon review, we realized that `return` at the end of a `with` block should be disallowed in the same way that `return` at the end of a method is disallowed.

This a breaking change. If you had code such as:

```pony
actor Main
new create(env: Env) =>
with d = Disposble do
d.set_exit(10)
return
end
```

You'll need to update it to:

```pony
actor Main
new create(env: Env) =>
with d = Disposble do
d.set_exit(10)
end
```

The above examples are semantically the same. This also fixes a compiler crash if you had something along the lines of:

```pony
actor Main
new create(env: Env) =>
with d = Disposble do
d.set_exit(10)
return
end
let x = "foo"
```

Where you had code "after the return" which would be unexpected by the compiler.
7 changes: 7 additions & 0 deletions src/libponyc/expr/control.c
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,13 @@ bool expr_return(pass_opt_t* opt, ast_t* ast)
return false;
}

if(ast_id(parent) == TK_DISPOSING_BLOCK)
{
ast_error(opt->check.errors, ast,
"use return only to exit early from a with block, not at the end");
return false;
}

ast_t* type = ast_childidx(opt->check.frame->method, 4);
ast_t* body = ast_child(ast);

Expand Down
1 change: 0 additions & 1 deletion test/full-program-tests/with-return/expected-exit-code.txt

This file was deleted.

20 changes: 0 additions & 20 deletions test/full-program-tests/with-return/main.pony

This file was deleted.

1 change: 1 addition & 0 deletions test/libponyc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ add_executable(libponyc.tests
use.cc
util.cc
verify.cc
with.cc
)

target_include_directories(libponyc.tests
Expand Down
33 changes: 33 additions & 0 deletions test/libponyc/with.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#include <gtest/gtest.h>
#include <platform.h>

#include "util.h"


#define TEST_COMPILE(src) DO(test_compile(src, "expr"))

#define TEST_ERRORS_1(src, err1) \
{ const char* errs[] = {err1, NULL}; \
DO(test_expected_errors(src, "expr", errs)); }

class WithTest : public PassTest
{};

TEST_F(WithTest, NoEarlyReturnFromWith)
{
// From issue #4464
const char* src =
"class Disposable\n"
" new create() => None\n"
" fun ref set_exit(x: U32) => None\n"
" fun dispose() => None\n"
"actor Main\n"
" new create(env: Env) =>\n"
" with d = Disposable do\n"
" d.set_exit(10)\n"
" return\n"
" end";

TEST_ERRORS_1(src,
"use return only to exit early from a with block, not at the end");
}

0 comments on commit b9f3b6b

Please sign in to comment.