ponyc icon indicating copy to clipboard operation
ponyc copied to clipboard

False positives in the type checking which identifies potential message sending classes.

Open adrianboyko opened this issue 3 years ago • 4 comments
trafficstars

use "collections"

actor Main
  new create(env: Env) =>
    None

  fun _final() =>
    Range(0,10)

Attempting to compile this code produces the following error:

         Launcher.pony:168:3: _final cannot create actors or send messages

         Launcher.pony:172:16: a message can be sent here
          for n in Range(0, size) do
                   ^

But Range is not an actor no does it send messages. Sean traced the issue to the following:

  // If we don't know the final type, we can't be certain of what all
  // implementations of the method do. Leave it as might send.
  if(!is_known(type)) {

Sean points out that the compiler will also reject instantiation of a union of two types, each of which are known to not send messages, even though the union type clearly won't send messages.

Joe says: "Yes this seems like a combination of (1) the check isn't as smart as it could/should be, to allow more cases that are "easily" proven safe, and (2) the error message implies too much certainty, and doesn't effectively communicate the possibility of a false positive"

adrianboyko avatar Nov 16 '22 20:11 adrianboyko

After debugging this i found out that the finalisers pass is analyzing the called function body and in this case is tripping over this line: https://github.com/ponylang/ponyc/blob/main/packages/collections/range.pony#L80

_forward = (_min < _max) and (_inc > 0)

It is complaining that it doesn't know the final return-type of the call to the function call _min.lt(_max) as those fields are of generic type A at this point.

One suggestion here is to actually reify the body of _final and then execute the check, as at this point all typeparamrefs should be resolved. Care should be taken, to only reify the body on its own without typeparams of the outer scope (function or entity type parameters). if any typeparams from the containing class/actor etc or function are still left, the types are really unknown.

mfelsche avatar Nov 17 '22 08:11 mfelsche

For this one case the error message should be more like "we can't determine if a message would be sent here".

SeanTAllen avatar Nov 22 '22 19:11 SeanTAllen

To differentiate the "can't determine" case from the "we know it will send" case, we could add another member to this enum: https://github.com/ponylang/ponyc/blob/0f50e3c5b7843bba5ccde083bf1a1e31fa7a400e/src/libponyc/pass/finalisers.c#L9-L14

jemc avatar Nov 22 '22 19:11 jemc

https://github.com/ponylang/ponyc/pull/4254 addresses the error message portion of this. We'd welcome folks coming along and improving the "dirt stupid" "is_known" check which is a good check but not sufficient for this use case. We need something more intelligent than "is_known" here.

SeanTAllen avatar Nov 22 '22 20:11 SeanTAllen