From 567df593f24a89af9ecdec7935003b36a3a98eba Mon Sep 17 00:00:00 2001 From: Hemanth Kapila Date: Fri, 11 Sep 2020 00:43:49 +0530 Subject: [PATCH 1/5] Prevent compiler crashes in `consume` expressions. * `consume (consume variable).field` must result in a similar compillation error as `consume (consume variable)`. * `consume f().field` must result in similar compilation error as `consume f()` * `(consume f).b` must be allowed and `f` should be moved. Fixes #3463, #3567 (and #3453) --- .release-notes/3650.md | 17 ++++++++++ src/libponyc/pass/refer.c | 8 ++--- src/libponyc/pass/syntax.c | 11 +++++-- test/libponyc/refer.cc | 66 +++++++++++++++++++++++++++++++++----- 4 files changed, 86 insertions(+), 16 deletions(-) create mode 100644 .release-notes/3650.md diff --git a/.release-notes/3650.md b/.release-notes/3650.md new file mode 100644 index 0000000000..099b5b846a --- /dev/null +++ b/.release-notes/3650.md @@ -0,0 +1,17 @@ +## Graceful error handling on `consume` expressions + +The fix ensures +* `consume (consume variable).field` results in a graceful compilation error +* `consume f().field` results in similar compilation error as `consume f()` +* Allow `(consume obj).field` expressions + The following snippet + ```pony + (consume f).b = recover Bar end + ``` + must be equivalent to + + ```pony + let x = consume f + x.b = recover Bar end + ``` + diff --git a/src/libponyc/pass/refer.c b/src/libponyc/pass/refer.c index 75ae6c64fc..9120b39d8a 100644 --- a/src/libponyc/pass/refer.c +++ b/src/libponyc/pass/refer.c @@ -139,6 +139,8 @@ static const char* generate_multi_dot_name(ast_t* ast, ast_t** def_found) { default: { + if (def == NULL) + return stringtab(""); pony_assert(0); } } @@ -146,7 +148,7 @@ static const char* generate_multi_dot_name(ast_t* ast, ast_t** def_found) { if(def_found != NULL) { *def_found = def; - + if(def == NULL) return stringtab(""); } @@ -1166,10 +1168,6 @@ static bool refer_consume(pass_opt_t* opt, ast_t* ast) "can't consume a let or embed field"); return false; } - } else if (ast_id(left) == TK_CALL) { - ast_error(opt->check.errors, ast, - "consume expressions must specify a single identifier"); - return false; } else { diff --git a/src/libponyc/pass/syntax.c b/src/libponyc/pass/syntax.c index f5fd638ee8..06da8db12c 100644 --- a/src/libponyc/pass/syntax.c +++ b/src/libponyc/pass/syntax.c @@ -696,14 +696,19 @@ static ast_result_t syntax_consume(pass_opt_t* opt, ast_t* ast) { case TK_THIS: case TK_REFERENCE: - case TK_DOT: return AST_OK; - + case TK_DOT: { + AST_GET_CHILDREN(term, left, right); + if (ast_id(left) != TK_CALL && ast_id(left) != TK_SEQ) + { + return AST_OK; + } + } default: {} } ast_error(opt->check.errors, term, - "Consume expressions must specify a single identifier"); + "Consume expressions must specify an identifier or field"); return AST_ERROR; } diff --git a/test/libponyc/refer.cc b/test/libponyc/refer.cc index 8c4267ba87..2ebe2b0e41 100644 --- a/test/libponyc/refer.cc +++ b/test/libponyc/refer.cc @@ -841,12 +841,62 @@ TEST_F(ReferTest, ConsumeParamVarSubfieldReassignSameExpressionReuse) TEST_F(ReferTest, ConsumeTupleAccessorOfFunctionValResult) { - const char* src = - "actor Main\n" - "new create(env: Env) =>\n" - "let a = \"I am a string\"\n" - "consume a.chop(1)._1"; - - TEST_ERRORS_1(src, - "consume expressions must specify a single identifier"); + const char* src = + "actor Main\n" + "new create(env: Env) =>\n" + "let a = \"I am a string\"\n" + "consume a.chop(1)._1"; + + TEST_ERRORS_1(src, + "Consume expressions must specify an identifier or field"); +} + +TEST_F(ReferTest, DoubleConsumeWithFieldAccessor) +{ + const char* src = + "class Problem\n" + "let _unwrap_me: U8 = 0\n" + "fun iso unwrap() =>\n" + "let this_iso: Problem iso = consume this\n" + "consume (consume this_iso)._unwrap_me"; + + TEST_ERRORS_1(src, + "Consume expressions must specify an identifier or field"); +} + +TEST_F(ReferTest, ConsumeAfterMemberAccessWithConsumeLhs) +{ + const char* src = + "actor Main\n" + "new create(env:Env val) =>\n" + "let f : Foo iso = recover Foo end\n" + "(consume f).b = recover Bar end\n" + "let x:Foo tag = consume f\n" + "class Foo\n" + "var b: Bar iso\n" + "new create() =>\n" + "b = recover Bar end\n" + "class Bar\n" + "new create() => None\n"; + + TEST_ERRORS_2(src, + "can't use a consumed local or field in an expression", + "consume must take 'this', a local, or a parameter"); +} + +TEST_F(ReferTest, MemberAccessWithConsumeLhs) +{ + const char* src = + "actor Main\n" + "new create(env:Env val) =>\n" + "let f : Foo iso = recover Foo end\n" + "(consume f).b = recover Bar end\n" + "class Foo\n" + "var b: Bar iso\n" + "new create() =>\n" + "b = recover Bar end\n" + "class Bar\n" + "new create() => None\n"; + + TEST_COMPILE(src); } From 7c84f1b816dd3523a217aca76e9a876a5120133b Mon Sep 17 00:00:00 2001 From: Hemanth Kapila Date: Mon, 14 Sep 2020 21:15:02 +0530 Subject: [PATCH 2/5] format fixes Co-authored-by: Joe Eli McIlvain --- src/libponyc/pass/syntax.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libponyc/pass/syntax.c b/src/libponyc/pass/syntax.c index 06da8db12c..4b69b1b534 100644 --- a/src/libponyc/pass/syntax.c +++ b/src/libponyc/pass/syntax.c @@ -698,11 +698,11 @@ static ast_result_t syntax_consume(pass_opt_t* opt, ast_t* ast) case TK_REFERENCE: return AST_OK; case TK_DOT: { - AST_GET_CHILDREN(term, left, right); - if (ast_id(left) != TK_CALL && ast_id(left) != TK_SEQ) - { - return AST_OK; - } + AST_GET_CHILDREN(term, left, right); + if (ast_id(left) != TK_CALL && ast_id(left) != TK_SEQ) + { + return AST_OK; + } } default: {} } From c08a8916c410d361fea0a1d1d1f1b86d809ac651 Mon Sep 17 00:00:00 2001 From: Hemanth Kapila Date: Mon, 14 Sep 2020 21:15:21 +0530 Subject: [PATCH 3/5] remove extra whitespace Co-authored-by: Joe Eli McIlvain --- src/libponyc/pass/refer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libponyc/pass/refer.c b/src/libponyc/pass/refer.c index 9120b39d8a..f20efb06d9 100644 --- a/src/libponyc/pass/refer.c +++ b/src/libponyc/pass/refer.c @@ -148,7 +148,6 @@ static const char* generate_multi_dot_name(ast_t* ast, ast_t** def_found) { if(def_found != NULL) { *def_found = def; - if(def == NULL) return stringtab(""); } From 11cb58b471d3674b3e77aec3b8f2b76fdb5c7129 Mon Sep 17 00:00:00 2001 From: Hemanth Kapila Date: Mon, 14 Sep 2020 23:19:38 +0530 Subject: [PATCH 4/5] Update .release-notes/3650.md Co-authored-by: Sean T Allen --- .release-notes/3650.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.release-notes/3650.md b/.release-notes/3650.md index 099b5b846a..a8a38e5d77 100644 --- a/.release-notes/3650.md +++ b/.release-notes/3650.md @@ -5,6 +5,7 @@ The fix ensures * `consume f().field` results in similar compilation error as `consume f()` * Allow `(consume obj).field` expressions The following snippet + ```pony (consume f).b = recover Bar end ``` @@ -14,4 +15,3 @@ The fix ensures let x = consume f x.b = recover Bar end ``` - From 3fb6649efd08af67476c4998b769230b9403c3b1 Mon Sep 17 00:00:00 2001 From: Hemanth Kapila Date: Mon, 14 Sep 2020 23:36:21 +0530 Subject: [PATCH 5/5] rephrasing the release notes --- .release-notes/3650.md | 20 ++++---------------- 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/.release-notes/3650.md b/.release-notes/3650.md index a8a38e5d77..6f6922dfd2 100644 --- a/.release-notes/3650.md +++ b/.release-notes/3650.md @@ -1,17 +1,5 @@ -## Graceful error handling on `consume` expressions +## Handling compiler crashes in some `consume` expressions -The fix ensures -* `consume (consume variable).field` results in a graceful compilation error -* `consume f().field` results in similar compilation error as `consume f()` -* Allow `(consume obj).field` expressions - The following snippet - - ```pony - (consume f).b = recover Bar end - ``` - must be equivalent to - - ```pony - let x = consume f - x.b = recover Bar end - ``` +Previously an expression of the form `consume (consume variable).field` resulted in the compiler crashing down with an assertion failure. With this fix, we get a friendly error message pointing to the erroneous line. + +Likewise, a valid assignment of the form `(consume f).b = recover Bar end` resulted in a compiler crash. With the fix, the compiler successfuly type checks the assignment and ensures that `f` is _consumed_.