Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generalize error constructors to mappings and lists #2

Closed
jclark opened this issue Mar 20, 2019 · 10 comments
Closed

Generalize error constructors to mappings and lists #2

jclark opened this issue Mar 20, 2019 · 10 comments
Assignees
Labels
Area/Lang Relates to the Ballerina language specification status/discuss Needs discussion outside github comments Type/Improvement Enhancement to language design

Comments

@jclark
Copy link
Collaborator

jclark commented Mar 20, 2019

Problem:
Error constructors use a function-like syntax in what seems to be a rather ad-hoc way

Solution:
Generalize error constructor (with changes from #2) into a functional constructor, which would be useable for both construction and pattern matching.

functional-constructor-expr := named-type-reference _(_ arg-list _)_
named-type-reference := variable-reference | error

The named-type-reference specifies the contextually expected type. The interpretation of arg-list depends on the basic type referred to:

  • for error, the argument list has an optional positional argument specifying the error reason string, and a named argument for each field in the detail record
  • for mappings, the argument list has a named argument for each field
  • for lists, the argument lists has a positional argument for each member

Note that new is restricted to behavioural types (matching the current impl).

@jclark jclark added this to the 2019R1 milestone Mar 20, 2019
@jclark jclark added Type/Improvement Enhancement to language design Area/Lang Relates to the Ballerina language specification labels Mar 24, 2019
@jclark jclark added status/pending Design is agreed and waiting to be added status/blocked Progress is blocked by another issue and removed status/pending Design is agreed and waiting to be added labels Mar 26, 2019
@jclark
Copy link
Collaborator Author

jclark commented Mar 26, 2019

Blocked by #27

@jclark jclark added status/pending Design is agreed and waiting to be added incompatible Resolving issue may not be backwards compatible and removed status/blocked Progress is blocked by another issue labels Apr 1, 2019
@jclark jclark removed the incompatible Resolving issue may not be backwards compatible label Apr 17, 2019
@jclark
Copy link
Collaborator Author

jclark commented Apr 17, 2019

This depends on issue #67, which was previously included in this.

@jclark
Copy link
Collaborator Author

jclark commented Jun 18, 2019

I have separated the error-specific part of this into #223, so this issue is just about generalizing that to mappings and lists.

@jclark jclark changed the title Generalize error constructors Generalize error constructors to mappings and lists Jun 18, 2019
@sanjiva
Copy link
Contributor

sanjiva commented Jun 18, 2019

I remain concerned that this syntax is going to be utterly confusing to Mort. Making new XML elements with that syntax will further make matters worse.

I would like to use new in front of it or some other verb like create to make clear its not a function call. I've been uneasy about error ("foo", { .. }) because of its syntactic similarity to function calls too but I've trained my Mort mind to look past it for the the known (and restricted) name error. But I shudder at this stuff:

type buy record { 
  int count; 
  string symbol; 
  float maxPrice; 
};

type sell record { 
  int count; 
  string symbol;
};

function execSale (sell r) { .. }
function execBuy (buy r) { .. }
function main () {
  var b = buy (10, "WSO2", 10551.0);
  execBuy (b);
}

If the type name is a verb then it becomes really messy to read and instantly see what that code is doing. You can argue that "well data types should not be named as verbs" but majority of the world's programmers (esp Morts) are going to be non-native English speakers and correct grammar is not a major requirement to be a programmer :-).

@jclark
Copy link
Collaborator Author

jclark commented Jun 18, 2019

Your example is confusing not merely because of the use of a verb rather than a noun, but because you are not using a naming convention to distinguish type names from other names. Given that type names share a symbol space with other names, it is crucial for clarity in Ballerina that type names using a naming convention to distinguish them, such as an initial capital letter or a _t suffix (like in C). (With hindsight, it might have been better if we had adopted this convention for our built-in non-simple types.)

So your example would more realistically be

var b = Buy(count = 10, symbol = "WSO2", maxPrice = 10551.0);

I disagree with the idea of using new in front, principally because these are constructors that are also used not only in expressions but also in binding patterns. We discussed this at length already.

At this point, I think our choices are to go ahead as designed for R2, or not. It's a feature we can live without for now. But in either case, I do think we need to do #223, otherwise our error handling will be very awkward.

@jclark jclark added status/discuss Needs discussion outside github comments and removed status/pending Design is agreed and waiting to be added labels Jun 18, 2019
@jclark
Copy link
Collaborator Author

jclark commented Jun 19, 2019

Another difficulty is that it would break the clear story that we currently on when new is used: it is used only to create objects. If you start to allow new for records, then you get a duplicate between a bare new and {}.

@sanjiva
Copy link
Contributor

sanjiva commented Jun 19, 2019

IMO the duplication of {} and new is ok - we actually had that a while ago too but I think you have a different view of that :).

I would prefer to not do this for R2/R3 and defer it to give it more consideration. I agree we need to go ahead with this for 223, but given error values are in their own value space I don't see that as necessarily saying the only way to address this issue is with the same syntax.

@jclark jclark removed this from the 2019R2 milestone Jun 20, 2019
@jclark jclark added this to the 2019R4 milestone Jun 20, 2019
@jclark
Copy link
Collaborator Author

jclark commented Jun 26, 2019

There is an interesting question about what the semantics of this in a match-pattern.

Suppose R is a record type and consider

match x {
R() => { var y = x; }
}

When does the match succeed? Is it when x looks like R (ie the current shape of x is in R) or is it when x belongs to R (ie the inherent type of x is a subtype of R)? I believe the right answer is the former, since match is about the current shape of things, not the type (the if clause checks type), and this is consistent with not narrowing the type of x in the match.

Note that for the case of error in #223 this does not arise, since errors are immutable meaning looks like and belongs to are the same for error.

@jclark jclark modified the milestones: 2019R5, 2020R1 Dec 21, 2019
@jclark jclark modified the milestones: 2020R1, 2020R2 Jan 23, 2020
@jclark
Copy link
Collaborator Author

jclark commented Mar 25, 2020

Relates to #152.

@jclark
Copy link
Collaborator Author

jclark commented Aug 6, 2020

Given the solution to #497, we are not doing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/Lang Relates to the Ballerina language specification status/discuss Needs discussion outside github comments Type/Improvement Enhancement to language design
Projects
None yet
Development

No branches or pull requests

2 participants