aeternity / aesophia

Stand alone compiler for the Sophia smart contract language
https://docs.aeternity.com/aesophia
ISC License
51 stars 19 forks source link

Polymorphism support #357

Closed ghallak closed 2 years ago

ghallak commented 2 years ago

Fixes: #307

ghallak commented 2 years ago

I have noticed that the type for the function declaration entrypoint init : () => unit is:

{fun_t,_,[],[],{id,_,"unit"}}

While the type of the function entrypoint init() = () is:

{tuple_t,_,[]}

The function aeso_ast_infer_types:unfold_types_in_type replaces {id, _, "unit"} with {tuple_t, _, []}, but it does not seems to be called on the fun_t type.

@hanssv Do you know if this is a bug or if it's done on purpose?

hanssv commented 2 years ago

I have no recollection of it being done on purpose...

radrow commented 2 years ago

Also, please refer some runtime tests in aeternity repo, so we are sure nothing breaks there.

ghallak commented 2 years ago

@hanssv @UlfNorell In the case of implementing two interfaces with entrypoints of the same name and same type

contract interface I =
    entrypoint f : () => int

contract interface J =
    entrypoint f : () => int

contract C : I, J =
    entrypoint f() = 1

Do you think this should pass as a function called f with the type () => int is already implemented? Or should it fail since only one of the interfaces is implemented, and not the other?

I already have this test and it's failing, but I'm wondering if it should.

UlfNorell commented 2 years ago

I think the least error prone option is to not allow implementing interfaces with overlapping entrypoints. So in this case it should give an error already on contract C : I, J =. I don't see any great harm in allowing it though, so if there are convincing use cases where you want overlapping interfaces we could accept it.

marc0olo commented 2 years ago

@ghallak I agree with Ulf, as long as there is no valid usecase to argue for we shouldn't allow implementing interfaces with overlapping entrypoints

radrow commented 2 years ago

I'd also modify some tests to involve tuples, maps, lists etc and custom types, so we are sure that subtyping works on them too

ghallak commented 2 years ago

~I should probably rename all new test files to have a common prefix related to this PR before merging it.~ (I have done that here https://github.com/aeternity/aesophia/pull/357/commits/e4db742c72b7e76f1ba72afdc29eae9b2b1d793f)

radrow commented 2 years ago

It's not perfect, but it's usable. We should consider improvements in the inference, but what is important now is: