ponyc
ponyc copied to clipboard
Default arguments aren't handled correctly on method calls of unions
If a method call is valid for all members of a union, it should be possible to call on the union.
This isn't true if the methods have a different number of parameters due to optional parameters. For instance, calling .string()
on a JsonType fails with an error:
Error:
/example/main.pony:12:12: a member of the union type has an incompatible method signature
x.string()
^
Info:
/somelocation/lib/pony/0.42.0/packages/json/json_type.pony:97:3: methods have a different number of parameters
fun string(indent: String = "", pretty_print: Bool = false): String =>
^
/somelocation/lib/pony/0.42.0/packages/builtin/real.pony:711:3: methods have a different number of parameters
fun string(): String iso^ =>
Even though the call .string() is valid on all members of the union
Can you give a minimal example?
I think I understand what you are saying and I don't think the example I believe you are giving is an error. A minimal example would be good.
Sure: https://playground.ponylang.io/?gist=d0c848aa83aeac85e93dfe5c11bd10ad
class A
fun string(): String => "A"
class B
fun string(excited: Bool = false): String =>
"B" + (if excited then "!" else "" end)
actor Main
new create(env: Env) =>
// fine
let a: A = A
env.out.print(a.string())
// fine
let b: B = B
env.out.print(b.string())
// error!
let ab: (A | B) = A
env.out.print(ab.string())
So this works and I think rightly so:
class Foo
fun string(x: String = "foo"): String iso^ =>
x.clone()
class Bar
fun string(x: String): String iso^ =>
x.clone()
actor Main
new create(env: Env) =>
let x: (Foo | Bar) = Foo
env.out.print(x.string("fred"))
and this doesn't:
class Foo
fun string(x: String = "foo"): String iso^ =>
x.clone()
class Bar
fun string(): String iso^ =>
"bar".clone()
actor Main
new create(env: Env) =>
let x: (Foo | Bar) = Foo
env.out.print(x.string())
And I think also rightly so. One is string() the other is string(string) and those are not the same method even if string(string) has a default value.
yeah @jasoncarr0 I don't think this is a bug. I think this is correct. A.string()
and B.string()
aren't same method. They have different signatures. I wouldn't consider those to have a subtype relationship.
If it was just subtyping I would be more amenable to agree (but still strongly prefer allowing it). But here it's a simple method call so it's far more surprising that we need to do this:
match x
| let f: Foo => f.string()
| let b: Bar => b.string()
end
I'd say it's still odd that something which supports .string()
isn't Stringable
. But in any case we'll discuss in sync
It doesn't support string()
it supports string(string)
.
When something with a default value is called, it is still string with 1 parameter. not string with no parameters. The sharing of a name does not make them the same. Return type needs to be the same, as does the number of parameters.
Here's example:
class Foo
fun string(x: String = "foo"): String iso^ =>
x.clone()
class Bar
fun string(): String iso^ =>
"bar".clone()
actor Main
new create(env: Env) =>
let x: (Foo | Bar) = Foo
env.out.print(x.string("foo"))
Foo.string is a fundamentally different than Bar.string. I think no one would argue that the example above should compile.
So it is again a default so really your example is:
match x
| let f: Foo => f.string("foo")
| let b: Bar => b.string()
end
Which I don't think is at all surprising. The existence of defaults do not change the method signature, they allow the writer to elide them from their code and have the compiler insert it for them.
I would be very much in favor of enhancing the tutorial to explain that default values in methods do not change the signature.
In the sync call, I agreed with Jason that conceptually the function with default arguments should be a subtype of the function without arguments. Sean does not agree.
Making it work as Jason and I expect is tricky, because default arguments are injected at the call site. We'd likely need to create a virtual table entry that takes zero arguments, but acts as a shim function that calls the real function and injects the default arguments within the virtual shim.
Then we'd need to work through updating the selector painting logic to correctly assign the right virtual table index for this case vs the concrete case without the shim.
So it would be non-trivial to make this work, and it's somewhat controversial at the moment (we don't yet agree that we'd like to implement this).
In the short term everyone agrees that we should update the Pony tutorial to document the limitation so that it doesn't catch people by surprise - it's just not agreed whether the limitation is a bug or an intentional limitation.
Small update #4088 is non-controversial and requires the same work as this to make it work so if someone wants to take on this work, it will be accepted. The question will be "what are the resolution rules".