Fix identity comparison check with desugared creations

[leonardoce]

While identity comparison with new objects was correctly catched by the
compiler as always false when using sugared syntax, it wasn't using
desugared one.

This patch makes the compiler in the latter case.

This example is now failing:

class C

actor Main
  new create(env: Env) =>
    env.out.print(if C.create() is C.create() then
      "C is C" else "C is NOT C" end)

Closes #4162

[SeanTAllen]

This looks like an excellent PR. I've left a couple comments on the release notes.

Assuming this passes CI, this has my approval.

[leonardoce]

Thanks @SeanTAllen for your review! I should have applied the suggestions you made.
I'm waiting for the CI to finish. Last time the x86-64 Windows MSVC platform failed with a timeout.

[SeanTAllen]

to go along with the missed cases that Joe mentions in his other comment, we need to also test with a constructor that isn't new.

So something like

class D
  new foo() => None

then as part of a test, that...

D.foo() is D.foo()

gives our error

For the sake of completeness, we should also the same with an actor and also an actor with a custom named constructor like foo.

[leonardoce]

Yes! The existence of constructors which aren't called new was even documented here: https://tutorial.ponylang.io/types/classes.html#constructors

I'm still a newbie when it comes to Pony. Ok, let me work on it.

[jemc]

Not every constructor has the name "create", and it is possible to create a function whose name is "create". So the name is not the right criterion to check here.

Instead of checking here based on the name of the called method, we should be checking if it refers to a constructor on the type.

Here in the refer.c pass, that would be a bit complicated (though still possible).

My suggestion is that you move this entire function to the verify.c pass, as well as the entire valid_is_comparand function and the refer_is function that calls it (which would become verify_is).

The reason is that in the expr.c pass we actually resolve every TK_DOT to be specific to what it is calling. So each TK_DOT that calls a constructor gets turned into a TK_NEWREF or TK_NEWBEREF (for the case of an actor constructor).

So if you move this set of checks into the verify.c pass, it becomes simple to check if the call is to a constructor - just check the AST ID for TK_NEWREF or TK_NEWBEREF. And in that case you don't need to bother checking the left side (the type reference) so it becomes a bit simpler than you have here, which is nice.

Also, conceptually, this check doesn't really fit the refer.c pass anyway, as it has nothing to do with resolving TK_REFERENCE nodes. Instead, it belongs in the verify pass, which has a variety of post-type-analysis correctness checks.

Here is the link to the original commit where these checks were added, in case it helps you see which parts need to be moved to verify.c: f37e281

[leonardoce]

Nice idea bringing this to the verify step. Let me try changing this PR with your suggestions. Thanks @jemc @SeanTAllen

[leonardoce]

Looks like GitHub allows to request a review by just one user, I think this is why my review request from @jemc removed the one for @SeanTAllen that I did previously. Sorry for the confusion.

I should have addressed your points. Can you please have a look? Thank you for reviewing this!

[SeanTAllen]

You can do more than 1, but the interface is wonky if you aren't used to it. Also, I see both of us as reviewers.

[jemc]

I only had the one small whitespace fix to mention above. Everything else looks okay 👍

[leonardoce]

I applied @jemc 's suggestion, squashed everything in one commit and rebased it on main.

[jemc]

@SeanTAllen - any further thoughts on this one?

[SeanTAllen]

@jemc i leave for 10 days in a day and have a lot of work stuff to get through so I leave this entirely in your capable hands. If you think it is good, I think it is good! I don't have time to look before I leave and I'd rather than be an impediment to this being merged whenever you think it is ready.

[jemc]

Thanks, @leonardoce!

[SeanTAllen]

Thank you very much!