ponyc icon indicating copy to clipboard operation
ponyc copied to clipboard

Inconsistent compiler behavior for Array literals in field initialization

Open stefandd opened this issue 2 years ago • 3 comments

The compiler is inconsistent regarding how Array fields of an embedded class can be initalized using Array literals. The following compiles fine:

class Foo
    embed _array: Array[I32] = []

actor Main
    let env: Env

    new create(env': Env) =>
        env = env'

whereas when _array is initialized to a non-empty literal:

class Foo
    embed _array: Array[I32] = [0]

actor Main
    let env: Env

    new create(env': Env) =>
        env = env'

it complains:

Error:
main.pony:2:5: an embedded field must be assigned using a constructor
    embed _array: Array[I32] = [0]
    ^
    Info:
    main.pony:2:32: the assigned expression is here
        embed _array: Array[I32] = [0]
                                   ^

@jemc proposes that both forms of initialization should however be allowed. Since [] is sugar for Array.create() and [0] is sugar for Array.create(1).>push(0), the compiler should allow both instead of rejecting the second.

The Zulip discussion is here: https://ponylang.zulipchat.com/#narrow/stream/189934-general/topic/Inconsistent.20use.20of.20Array.20literals.20in.20field.20initialization

stefandd avatar Aug 10 '22 21:08 stefandd

The first step would be to modify the following https://github.com/ponylang/ponyc/blob/99a1c6a83e2ba7acb179bbd51b27ee2cc462d745/src/libponyc/expr/operator.c#L212-L218

To deal with TK_FUNCHAIN (which represents a series of calls with.>):

diff --git a/src/libponyc/expr/operator.c b/src/libponyc/expr/operator.c
index e7863690..735681bf 100644
--- a/src/libponyc/expr/operator.c
+++ b/src/libponyc/expr/operator.c
@@ -215,7 +215,7 @@ static bool is_expr_constructor(ast_t* ast)
   switch(ast_id(ast))
   {
     case TK_CALL:
-      return ast_id(ast_child(ast)) == TK_NEWREF;
+      return (ast_id(ast_child(ast)) == TK_NEWREF) || (ast_id(ast_child(ast)) == TK_FUNCHAIN);
     case TK_SEQ:
       return is_expr_constructor(ast_childlast(ast));
     case TK_IF:

But it seems like that doing only that isn't enough. See the results of changing only that:

class Foo
  embed array: Array[U8] = []

  new create() => None

class Bar
  embed array: Array[U8] = [0]

  new create() => None

actor Main
  new create(env: Env) =>
   env.out.print(Foo.array.size().string()) // Prints `0`
   env.out.print(Bar.array.size().string()) // Prints `4508837504`

So there might be other parts during code generation that need to be adjusted.

ergl avatar Aug 11 '22 10:08 ergl

@stefandd are you interested in tackling this?

SeanTAllen avatar Oct 18 '22 18:10 SeanTAllen

@SeanTAllen As much as I'd like to, I still know far too little about the compiler as of yet to even know where to look for a solution. On the other hand, @ergl knew where to look and what to adjust, so I'd hope for him to succeed hunting this down all the way...

stefandd avatar Oct 27 '22 14:10 stefandd