ballerina-platform / ballerina-spec

Ballerina Language and Platform Specifications
Other
168 stars 53 forks source link

Usage of typeParams in xml langlib functions #1319

Open poorna2152 opened 2 weeks ago

poorna2152 commented 2 weeks ago

Description: Based on the spec, the type defined by the typeParam is defined to be an upper bound on the type parameter.

When a module type definition has a @typeParam annotation, it means that this type serves as a type parameter 
when it is used in a function definition: all uses of the type parameter in a function definition 
refer to the same type; the definition of the type is an upper bound on the type parameter.

In the case of xml consider the following forEach function definition.

public isolated function forEach(xml<ItemType> x, @isolatedParam function(ItemType item) returns () func)
    = external;

Where the ItemType is defined as,

@typeParam
type ItemType Element|Comment|ProcessingInstruction|Text;

The second parameter func in the forEach function defines a function which accepts parameter item of type ItemType.

Consider following examples,


xml<xml:Element> a = xml `<a/><b/>`;

// This works because the `ItemType` is actually an upper bound for `xml:Element`
a.forEach(function (xml:Element e) {

});

xml b = xml `<a/><b/>`;
// Based on the spec this shouldn't work because the `ItemType` is not an upper bound  `xml`
b.forEach(function (xml e) {

});

With the definition of typeParam and the forEach function in the spec second case is invalid. This is because the in the second case the ItemType is not an upper bound for xml. This worked so far in the compiler due to a bug. Shouldn't the definition in the spec be modified to support this case as well?

Suggested Labels:

Code sample that shows issue:

Related Issues:

jclark commented 2 weeks ago

What are you suggesting it be modified to?

poorna2152 commented 2 weeks ago

Both myself and @heshanpadmasiri had a discussion on this and what we thought was about was the following three options.

  1. Define the forEach and such langlib function to accept a union with non typeParam type as well

    public isolated function forEach(xml<ItemType> x, @isolatedParam function(ItemType item) returns ()|function(Element|Comment|ProcessingInstruction|Text item) returns () func)
  2. Change the definition of typeParams.

  3. Define a new annotation for this usage.

Out of these I think the first one is the better option.

jclark commented 2 weeks ago

I'm still not sure I understand why this is invalid:

xml b = xml `<a/><b/>`;
// Based on the spec this shouldn't work because the `ItemType` is not an upper bound  `xml`
b.forEach(function (xml e) {

});

Let's define

type X function(Element|Comment|ProcessingInstruction|Text item) returns ();
type Y function (xml item) returns ();

If we instantiate the type of forEach based on the type of b, it would expect an argument that is a subtype of X (since xml is the same as xml<Element|Comment|ProcessingInstruction|Text>). But we are providing it with a function of type Y and Y is a subtype of X (since functions are covariant in their parameter types and Element|Comment|ProcessingInstruction|Text is a subtype of xml).

Am I confused?

poorna2152 commented 2 weeks ago

Here the function is passed a parameter of type ItemType which is a typeParam.

@typeParam
type ItemType Element|Comment|ProcessingInstruction|Text;

When a parameter is marked as a typeParam it defines an upper bound for the type that can be passed to that. Here Element|Comment|ProcessingInstruction|Text is not an upper bound for xml. And also when a parameter is defined as a typeParam we do not check the covariant right.