ponylang / ponyc

Pony is an open-source, actor-model, capabilities-secure, high performance programming language
http://www.ponylang.io
BSD 2-Clause "Simplified" License
5.71k stars 415 forks source link

Compiler crash when using `as` on numeric literal #2037

Closed ta3ta1 closed 7 years ago

ta3ta1 commented 7 years ago

When compiling following code,

actor Main
  new create(env: Env) =>
    try
      let n = 0 as I64
    end

compiler crashes and shows stacktrace.

e:\github\ponyc\src\libponyc\type\matchtype.c:403: is_x_match_nominal: Assertion `0` failed.

Backtrace:
  (ponyint_assert_fail) [00007FF7D110D11D]
  (is_x_match_nominal) [00007FF7D10F0731]
  (is_x_match_x) [00007FF7D10EF78E]
  (is_matchtype) [00007FF7D10EF6A7]
  (add_as_type) [00007FF7D108E8BC]
  (expr_as) [00007FF7D108CC08]
  (pass_expr) [00007FF7D105A195]
  (ast_visit) [00007FF7D0FF837C]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (ast_visit) [00007FF7D0FF82BE]
  (visit_pass) [00007FF7D0FF869F]
  (ast_passes) [00007FF7D0FF89A8]
  (ast_passes_program) [00007FF7D0FF7E93]
  (program_load) [00007FF7D100DB7A]
  (compile_package) [00007FF7D0FEE827]
  (main) [00007FF7D0FEED50]
  (__scrt_common_main_seh) [00007FF7D2873CA5]
  (BaseThreadInitThunk) [00007FFA75E22774]
  (RtlUserThreadStart) [00007FFA76D80D61]

Expected Result: No crash at least. I'm not sure what should be done.

Compile version: debug build from 2ca91f9ca30454070926124d4d84a23959c497ee

0.15.0-6a50fb290 [debug]
compiled with: llvm 3.9.1 -- msvc-15-x64
ta3ta1 commented 7 years ago

Using as operator on float literal also produce same result.

actor Main
  new create(env: Env) =>
    try
      let n = 0.0 as F64
    end
jemc commented 7 years ago

We need to fix the crash, probably in a similar way to the solution in #2035.

But from a larger perspective, we should probably add a check to the syntax pass that looks for a numeric literal on the left side of an as operator, and gives a compiler error telling you that this isn't how you give the inference hint for a numeric literal - the correct pattern is to use I64(0), not 0 as I64.

ta3ta1 commented 7 years ago
actor Main
  new create(env: Env) =>
    try
      if true then
        1
      else
        2
      end as I64
    end

This also crash. I'll update PR #2041, adding a check to expr pass.

Praetonus commented 7 years ago

@jemc I'd argue that 0 as I64 should be allowed as a mean to specify a literal type. It's consistent with the explicit array syntax ([as I64: 0; 1; 2]).

jemc commented 7 years ago

@Praetonus - if we allowed that, would it be an "error-raising" operation?

Praetonus commented 7 years ago

@jemc That's an interesting point.

One possibility would be to make as an error-raising operation only when the resulting match would be non-exhaustive (i.e. when the LHS type isn't equivalent to the RHS type). Specifying literal types would probably be the only interesting operation with equivalent types on both sides of an as, but at least we'd have a consistent rule.

jemc commented 7 years ago

Yeah, that's one option, though it does seem to go against the spirit of #1771, the purpose of which is to make it obvious to the reader from alone syntax where an error may be raised.

Praetonus commented 7 years ago

Good point, I think that's a compelling argument against that new rule for as, and by extension against specifying literal types with as.

Then the correct fix would be to issue a compiler error in expr_as if the LHS type is a literal type.

jemc commented 7 years ago

Then the correct fix would be to issue a compiler error in expr_as if the LHS type is a literal type.

Agreed - we need to fix it there so that it covers all cases. However, I do think the change in #2041 would be a good additional measure, to provide a more helpful error message for the common, confusing case.

That is, I think we should do both - that's what I was getting at with this comment: https://github.com/ponylang/ponyc/issues/2037#issuecomment-315088733

SeanTAllen commented 7 years ago

2041 should fix this.