Unreported type inference error with Array's in match case statements

[redvers]

Edited to put the minimal case up top instead of in the comments.

Running ponyc 0.54.0
Minimal case:

actor Main
  new create(env: Env) =>
    let t: Array[U8] val = [ 0x00 ; 0x01 ; 0x02 ]
    match t
    | [ 0x00 ; 0x00 ; 0x00 ] => false
    | [ 0x00 ; 0x01 ; 0x02 ] => true
    end

Here's how it looks:

[nix-shell:~/projects/pony/ssssss]$ ponyc --debug -r=refer
Building builtin -> /nix/store/nnvr2l04kdk62n633g74p0vdwf9bbrmy-ponyc-0.54.0/lib/pony/0.54.0/packages/builtin
Building . -> /home/red/projects/pony/ssssss

[nix-shell:~/projects/pony/ssssss]$ ponyc --debug -r=expr
Building builtin -> /nix/store/nnvr2l04kdk62n633g74p0vdwf9bbrmy-ponyc-0.54.0/lib/pony/0.54.0/packages/builtin
Building . -> /home/red/projects/pony/ssssss
Error: internal failure not reported

[SeanTAllen]

Here's a more minimal example that gives an error:

class Fu
  fun bar(t: Array[U8] val) =>
    match t
    | [ U8(1) ] => false
    end

which results in:

main.pony:4:7: couldn't find 'eq' in 'Array'
    | [ U8(1) ] => false
      ^
Error:
main.pony:4:7: this pattern element doesn't support structural equality
    | [ U8(1) ] => false
      ^

If you remove the type info for the array:

class Fu
  fun bar(t: Array[U8] val) =>
    match t
    | [ 1 ] => false
    end

you get the original error message which is the default "it says there was an error but we got no message for it" message.

You can replicate this with the original example by making the following changes:

actor Main
  new create(env: Env) =>
    let t: Array[U8] val = [ 0x00 ; 0x01 ; 0x02 ]
    match t
    | [ U8(0x00) ; U8(0x00) ; U8(0x00) ] => false
    | [ U8(0x00) ; U8(0x01) ; U8(0x02) ] => true
    end

The missing error message is likely some form of "could not infer literal type, no valid types found" that you get when you do something like:

class Fu
  fun bar() =>
    let x = [ 1 ]

but in this case, in the match, it appears we don't give a nice error message for it.

[redvers]

I wondered if eq ended up involved somehow (and part of the reason I asked about that in a separate thread on zulip).

That makes sense. Thanks.

So this is going to be a "turn into a compile-time error" type of solution I expect since implementing literal arrays in a match is a problem that is significantly bigger than a breadbox.

[SeanTAllen]

Note this is reproducible with Pony 0.54.1

[SeanTAllen]

So this is going to be a "turn into a compile-time error" type of solution I expect since implementing literal arrays in a match is a problem that is significantly bigger than a breadbox.

I'm not sure what you mean by "implementing literal array in a match". That isn't the bug here, the bug is that you aren't getting the error message that you should.

[SeanTAllen]

Note this is a variation of #2110 and it appears we missed a case.

[SeanTAllen]

This is hitting

  if(is_typecheck_error(pattern_type))
    return NULL

in match.c

https://github.com/ponylang/ponyc/blob/main/src/libponyc/expr/match.c#L56

We should never hit a typecheck error here, that it should have been caught before this point and the check is only a sanity check. This should have been caught and handled before we got to the make_pattern_type function.

In this particular case, pattern_type is coming back as null.

So, the preceeding ast_t* pattern_type = ast_type(pattern) is returning null, which I really wasn't expecting.

This is happening because

ast->annotation_type;

returns NULL in ast_type so the issue is, why is annotation_type NULL.

Based on everything here, I'm fairly confident that the "could not infer literal type, no valid types found" should have caught this already and we would never get here.

[SeanTAllen]

Note when using a debug version of the compiler you will get:

/home/sean/code/ponylang/ponyc-3/src/libponyc/pass/expr.c:661: pass_expr: Assertion `errors_get_count(options->check. Errors) > 0` failed.

as the error message.

[SeanTAllen]

Looking in expr_case, I see the following:

  if(ast_checkflag(match_expr, AST_FLAG_JUMPS_AWAY) ||
    is_typecheck_error(match_type))
    return false;

and the jumps away never gets handled until make_pattern_type:

  if(ast_checkflag(pattern, AST_FLAG_JUMPS_AWAY))
  {
    ast_error(opt->check.errors, pattern,
      "not a matchable pattern - the expression jumps away with no value");
    return NULL;
  }

so I think for this code it is reasonable that we need to note that we don't have a type as a check after the jumps away check. But I'd like to discuss with @jemc.

[jemc]

My suggestion:

In the find_antecedent_type function in expr.c, add another case to the switch that shows it how to find the proper antecedent if it determines that the AST in question is a match case pattern expression (which I think would be noticing that the ast_parent is a TK_CASE), and in that situation it could find the appropriate antecedent - the expression passed in to be matched (in this case, the variable t).