search

chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 420 forks source link

What does an undecorated class type `C` mean? #12917

Closed mppf closed 5 years ago

mppf commented 5 years ago

PR #12916 will change new MyClass from meaning new borrowed MyClass to meaning new owned MyClass.

This brings up several questions about what MyClass means when it is not decorated at all.

Introducing the Examples

There are two important examples for discussion.

Example 1: Declaring a Variable

var x: MyClass = new MyClass();

What is the type of x?

Example 2: new t for a type from an argument

proc foo(type t) {
  var y = new t();
  return y;
}
foo(MyClass);
foo(borrowed MyClass);

What is the type of y? What is the type of the calls to foo?

When new MyClass meant new borrowed MyClass - before PR #12916

var x: MyClass = new MyClass();

meant the same as var x: borrowed MyClass = new borrowed MyClass(); (by design).

proc foo(type t) {
  var y = new t(); // y has type `borrowed MyClass`
  return y; // error, can't return a borrow
}
foo(MyClass); // expression type is borrowed MyClass
foo(borrowed MyClass); // expression type is borrowed MyClass

But what should we do after new MyClass means the same as new owned MyClass ?

Proposals for var x: MyClass = new MyClass()

Note that after PR #12916, there may be an inconsistency in type inference between these two:

var x = new C();
var y:C = new C();

since x will have type owned C but y has type borrowed C.

Option 1: MyClass still means borrowed MyClass

var x: MyClass = new MyClass();

means the same as var x: borrowed MyClass = new owned MyClass();. This statement is legal and coerces from owned to borrowed. This does not resolve the type inference inconsistency but it may be acceptable.

Option 2: MyClass is a class with unspecified ownership, analogous to a generic type

var x: MyClass = new MyClass();

is conceptually similar to var x: SomeGenericRecord = new SomeGenericRecord(1,2,...); which is legal and results in x having type inferred from the new expression. In particular, in this case, it would lead one to expect that var x:MyClass = new owned MyClass has type owned MyClass. The example above, var x: MyClass = new MyClass(); would be just the same - meaning that x has type owned MyClass. In that way it would resolve the type inference inconsistency.

Option 3: MyClass means borrowed MyClass in some contexts and owned MyClass in others

var x: MyClass = new MyClass();

Here we might simply make a rule that undecorated MyClass, when used in a variable's type expression, becomes owned MyClass. This would also resolve the type inference inconsistency.

The challenge here is what about something like var y: f(MyClass)... but that gets closer to the second example.

Note that this idea is similar to an idea that was investigated and discussed a little bit before in https://github.com/chapel-lang/chapel/issues/8653

Proposals for foo(MyClass)

Here it comes down to choosing what it means to pass MyClass as a type argument and how the new t behaves.

Option A: un-decorated new means owned

proc foo(type t) {
  var y = new t(); // y has type `owned MyClass`
  return y; // ok
}
foo(MyClass); // expression type is owned MyClass
foo(borrowed MyClass); // expression type is owned MyClass

This behavior might be reasonable if we interpret MyClass to always mean borrowed MyClass, so that even new owned MyClass is actually equivalent to new owned borrowed MyClass, where the outermost management strategy wins, and here we imagine that the compiler simply changes un-decorated new into new owned.

Another way to describe it is that this proposal views new borrowed MyClass as a syntactic convenience for (new owned MyClass()).borrow() and does not consider that syntactic convenience something that should be communicated by type arguments.

I think this strategy makes the most sense of the options if we were to decide to deprecate new borrowed MyClass (Some users have reported confusion and distaste for new borrowed MyClass as a pattern at all). Additionally, it has the potential advantage that new t results in the more capable type (owned).

Option B: borrowed MyClass is a different type from MyClass

proc foo(type t) {
  var y = new t();
  return y; // error if t is borrowed
}
foo(MyClass); // expression type is owned MyClass
foo(borrowed MyClass); // expression type is borrowed MyClass

Here we might imagine that foo can be instantiated with a partially specified type (MyClass) and with a fully specified type borrowed MyClass).

The challenge is that most of the time we don't want the compiler to considerMyClass and borrowed MyClass to be different types; e.g. in a where clause.

(an example of such a where clause, as an aside:

class MyClass { var x: int; }

record Container {
  type eltType;
  var element:eltType;
}

proc merge(a:Container, b:Container) where a.eltType == b.eltType {
}

var aClass:MyClass = new borrowed MyClass(2);
var aBorrowedClass:borrowed MyClass = new borrowed MyClass(1);

var a = new Container(MyClass, aClass);
var b = new Container(borrowed MyClass, aBorrowedClass);
merge(a,b);

)

Option C: un-decorated MyClass means borrowed MyClass

proc foo(type t) {
  var y = new t();
  return y; // error, can't return borrowed MyClass
}
foo(MyClass); // expression type is borrowed MyClass
foo(borrowed MyClass); // expression type is borrowed MyClass

Here the idea would be that MyClass is, at least in expression contexts, equivalent to borrowed MyClass and therefore new t would be the same as new borrowed MyClass. However this might be undesirable since new borrowed MyClass is not as capable (e.g. it can't be returned).

Option D: can't pass un-decorated MyClass as a type argument

proc foo(type t) {
  var y = new t();
  return y; // error, can't return borrowed MyClass
}
foo(MyClass); // error, `MyClass` is "generic" since it doesn't specify ownership
foo(borrowed MyClass); // expression type is borrowed MyClass
benharsh commented 5 years ago

I'm leaning towards the options where 'MyClass' on its own is sort-of generic, in that it doesn't state 'borrowed' or 'owned' either way.

I think it would be OK to pass 'MyClass' to type formals and let 'new' use the managed type if present and 'owned' if it's generic.

mppf commented 5 years ago

@ben-albrecht @benharsh - what about

var x: MyClass;

Right now with a generic record, this would be an error. We might imagine MyClass to be like a generic with defaults, where it can become borrowed MyClass whenever a decision needs to be made. Alternatively, we might rather it become owned MyClass in this case. What do you think?

BryantLam commented 5 years ago

I'm concerned that the "sort-of generic" approach with MyClass might muddy the waters for what already feels like a clear Design.

D1. new without a managed intent === new owned D2. type MyClass === borrowed MyClass --> D3. new borrowed MyClass ==D2=> new MyClass ==D1=> new owned MyClass

From D1 and D2, the standard managed intents are only owned and shared. I'm inclined to treat borrowed as the explicit synonymous form of the undecorated MyClass.*

From D3, new borrowed equals new owned in generic contexts, and can thus be forbidden in concrete contexts (i.e., the user shouldn't be allowed to type new borrowed explicitly).

My strongest preference is for Option 1 + Option A' (amended to follow D3). The user only has to learn two rules: D1 and D2. D3 is a synthesis of the other two. I do admit that Option 1 is the most confusing to someone that doesn't know about D2.

--

*The alternative is to do the sort-of generic approach. But it definitely has some oddities inside classes.

class C {
  var x: MyClass; // Generic. This type is not fully specified.
  var y: MyClass;

  // If you want it to be a concrete `borrowed MyClass`, force the user to say it explicitly.†
  // Or let it be inferred from the assigned:
  proc init(yy: MyClass) {
    this.x = new owned MyClass(); // this.x is now owned MyClass
    // yy is not generic; it is a borrowed MyClass.
    // †This is slightly confusing to realize when something is or is not a generic.
    this.y = yy; // this.y is now borrowed MyClass.
  }
}

/* Option 2 */
var x: MyClass = new owned MyClass(); // x is owned MyClass

proc foo(type t) {
  var y = new t();
  return y;
}
/* Option A */
foo(MyClass); // From D1, returns owned MyClass. Though slightly confusing now; is this not fully specified?
foo(borrowed MyClass); // From D3, returns owned MyClass

var z: MyClass; // Today, Error: type not fully specified.
// z = new MyClass(); // Someday, z could be owned MyClass

I didn't think too much about the generic approach, so I probably missed something.

mppf commented 5 years ago

@BryantLam -

My strongest preference is for Option 1 + Option A' (amended to follow D3)

I'm having trouble seeing the difference between Option A and Option A'. Isn't the behavior in the example the same in A and A1 (i.e. foo(MyClass) and foo(borrowed MyClass) are both result in the type owned MyClass)? Are you just saying you also want expressions like literally new borrowed MyClass() to result in a compilation error?

BryantLam commented 5 years ago

@mppf -- I think part of my confusion is that your Option A has two subproposals that depend on whether new borrowed MyClass means new owned MyClass or (new owned MyClass()).borrow(). (I think you advocated the former as well.)

I chose the former by preferring the more capable new owned MyClass (D3). This choice provides the option to now explicitly forbid new borrowed MyClass because it's redundant and because borrowed isn't a managed intent compatible with new by my definition of D1.

Edit: borrowed probably should continue to be a managed intent, but it shouldn't be compatible with new. I'm confusing myself with the subsequent post about generic MyClass. Ignore this striken statement.

BryantLam commented 5 years ago

For a generic MyClass, the case with classes is more trickier than I thought. Here's another valid view:

class C {
  var x: MyClass; // Generic. This type is not fully specified.
  var y: MyClass;

  // If you want it to be a concrete `borrowed MyClass`, force the user to say it explicitly.†
  // Or let it be inferred from the assigned:
  proc init(yy: MyClass) {
    this.x = new owned MyClass(); // this.x is now owned MyClass
    // ~~yy is not generic; it is a borrowed MyClass.~~
    // yy is a generic!
    // †This is slightly confusing to realize when something is or is not a generic.
    this.y = yy; // this.y's type is inferred from yy.
  }
}

Thus, when would MyClass mean borrowed MyClass? I assume somewhere in the context-giving codebase, borrowed MyClass has to be said explicitly? Or do you continue to let owned MyClass coerce into a borrowed MyClass for all cases like the above C.init(yy)? I think this design might be more confusing than it is worth.

Edit: This post explores what if MyClass was allowed to be a kind of generic when not specified with a managed intent and assuming that var x: MyClass is using today's default of being a borrowed. With it being as confusing as it is, it might be better in the short term to just force the user to have to say borrowed or owned explicitly until we can settle on the correct default, if ever.

bradcray commented 5 years ago

While I think that most of the approaches above for interpreting foo(MyClass) can be justified, it seems to me that most users, given a function like this:

proc bar(type t) {
  var x: t;
  var y = new t();
}

would be surprised if calling it like foo(MyClass) were to behave differently than:

proc bar() {
  var x: MyClass;
  var y = new MyClass();
}

or if calling it as foo(borrowed MyClass) were to behave differently than:

proc bar() {
  var x: borrowed MyClass;
  var y = new borrowed MyClass();
}

This makes me nervous about options A and C since they seem to break this naive interpretation (i.e., I think that they would lead to user confusion if they could be avoided). It also makes me wonder if the midpoint between options B and D is the idea of MyClass being a generic type where the default is filled in when needed as Michael suggested above.

I'm ignoring the "what does it mean to declare a variable of type MyClass?" part of this issue for now because it seems less immediately concerning to me than this part.

mppf commented 5 years ago

@BryantLam explored how the generic strategy if applied to field declarations could lead to confusion. I think that's a reasonable concern, but it seems more connected to variable/field declarations than to the behavior of new t with a type argument t.

it might be better in the short term to just force the user to have to say borrowed or owned explicitly until we can settle on the correct default, if ever.

At first I thought this was referring to all uses of MyClass, but now I'm starting to think this might apply specifically to the type t case:

proc foo(type t) {
  var y = new t();
  return y;
}

What if we were to make new t like this be an error, and require that the new be decorated (i.e. new owned t() / new borrowed t() / etc? (Without impacting variable declarations at all - that is, still allowing var x:MyClass; or var x:t as we do now?).

I continue to view this situation as a corner case and I'm concerned that treating MyClass as a generic type (that becomes borrowed MyClass if it needs to) will be challenging in the near-term. In particular because I think there will be significant work to get a type alias to allow a generic type to be passed through. (We might undertake this work as part of the partial instantiations effort). Additionally, I think I would have a different answer here if we started to disallow new borrowed - which I can't yet predict if we will or won't do - so making it an error is appealing to me for the moment.

I'm ignoring the "what does it mean to declare a variable of type MyClass?" part of this issue for now because it seems less immediately concerning to me than this part.

These seem to be related, though. For example, if we were to take the option that MyClass is treated like a generic with defaults, where the default of borrowed MyClass isn't used until necessary, then wouldn't we necessarily also change the behavior of declarations like var x: MyClass = new owned MyClass(); ? (So that x would now be an owned MyClass rather than borrowed MyClass - this is Option 2 above).

If you agree that making undecorated new t simply an error is acceptable for now - then it seems that the question of variable declarations is the main question we need to settle here in order to be able to promise backwards compatibility.

bradcray commented 5 years ago

These seem to be related, though.

I don't mean to imply that they're not. Really what I mean is that I could imagine being comfortable in a world where var x: MyClass means either borrowed, owned, generic, or an error, so it feels like there are lots of degrees of freedom in the design there. Meanwhile I'm having big hesitations about passing an apparent type into a type alias and having it behave differently than if the type alias were just replaced with the actual type. That's why I'm focusing on that part of the question first.

One other case to consider is:

foo(shared MyClass)

where it seems like it would be very surprising to have the new t result in an owned in this case (which I think is what option A would do?).

What if we were to make new t like this be an error,

When you say "like this" do you mean "when new is applied to a type alias only" or "when any undecorated class flavor is applied to it"? (i.e., would new MyClass() also be an error?)

We might undertake this work as part of the partial instantiations effort

I definitely agree that the two are linked and wouldn't imagine we'd get the generic behavior without having completed the partial instantiation work as well.

mppf commented 5 years ago 
foo(shared MyClass)

where it seems like it would be very surprising to have the new t result in an owned in this case (which I think is what option A would do?).

What we have implemented now would create a shared in this case. It's debateable what Option A's description above says about it, but in preparing the description, I was focused on owned vs borrowed.

What if we were to make new t like this be an error,

When you say "like this" do you mean "when new is applied to a type alias only" or "when any undecorated class flavor is applied to it"? (i.e., would new MyClass() also be an error?)

when new is applied to a type alias only

Edit: e.g. the error could be "new applied to a type alias must currently be decorated"

bradcray commented 5 years ago

I'd be open to making it an error to apply new to an undecorated type alias if it helps us make forward progress, though I think it's a little unfortunate from the perspective of writing tests (e.g., I could imagine wanting to pass owned C, shared C, unmanaged C into a routine via a type alias to test various capabilities, and believe that we have tests that do so today). If we were to prevent MyClass from being bound to a type alias / passed into a type argument, would that give us the equivalent benefits without removing the test-oriented capability?

mppf commented 5 years ago

If we were to prevent MyClass from being bound to a type alias / passed into a type argument, would that give us the equivalent benefits without removing the test-oriented capability?

One (minor) thing it's not as easy to implement. But the other issue is that it would actually rule out some of the possible behaviors (in particular, Option A if it's interpreted with maximum strictness, might expect new t to result in owned MyClass even when passed unmanaged MyClass). But maybe those possibilities can be pruned.

bradcray commented 5 years ago

Option A if it's interpreted with maximum strictness, might expect new t to result in owned MyClass even when passed unmanaged MyClass

I'd definitely like to propose removing this option+interpretation from the table. Given the following code:

proc factory(type t) {
  return new t();
}

var a = factory(owned MyClass);
var b = factory(shared MyClass);
var c = factory(unmanaged MyClass);

If a, b, and c all had type owned MyClass, it's hard for me to imagine that users would expect this or find it useful. I think it'd be more surprising and hard to explain than anything.

bradcray commented 5 years ago

Tagging @BryantLam since he was a proponent of some form of option A. Bryant, I think it's easier to rationalize a "new t() should always generate owned t" rule when we're only considering owned and borrowed cases. But having opened the door to thinking about shared and unmanaged cases, does your support for option A go as far as believing that a, b, and c should all be owned in the example from the previous comment? (again, I can see a way to rationalize it, I just think users will find it far more confusing than useful / valuable).

The reason I'm trying to shift the discussion from making new t() illegal to making passing [undecorated] MyClass into type t illegal is that owned MyClass, shared MyClass, and unmanaged MyClass all seem like obviously complete type specifications whereas undecorated classes are less obviously so. So it seems like the proposal punishes the well-formed cases in service to the less well-defined one, which seems backward to me.

BryantLam commented 5 years ago

The reason I'm trying to shift the discussion from making new t() illegal to making passing [undecorated] MyClass into type t illegal is that owned MyClass, shared MyClass, and unmanaged MyClass all seem like obviously complete type specifications whereas undecorated classes are less obviously so.

I completely agree. To make this discussion more concrete, I see merit for only these two and a half approaches, of which I will name these approaches explicitly:

undecorated MyClass is always a borrow

Here, I am technically mixing two proposals:

var x1 = new owned MyClass();      // owned MyClass
var x2 = new shared MyClass();     // shared MyClass
var x3 = new unmanaged MyClass();  // unmanaged MyClass
var x4 = new borrowed MyClass();   // owned MyClass**
var x5 = new MyClass();            // owned MyClass (default undecorated new)

// Well, a user that does not know ownership would technically be confused
// by all five of the following lines, so these lines would rationalize for
// an approach where MyClass was this partially instantiated generic, but
// I don't want to go down that route for this proposal. If someone wants
// to counter-propose, be my guest, but I've already confused myself once...

var y1: MyClass = new owned MyClass();      // borrowed MyClass (from owned MyClass)
var y2: MyClass = new shared MyClass();     // borrowed MyClass (from shared MyClass)
var y3: MyClass = new unmanaged MyClass();  // borrowed MyClass (from unmanaged MyClass)
var y4: MyClass = new borrowed MyClass();   // borrowed MyClass (from owned MyClass**)
var y5: MyClass = new MyClass();            // borrowed MyClass (from owned MyClass)

proc factory(type t) {
  return new t();
}

var a1 = factory(owned MyClass);      // owned MyClass
var a2 = factory(shared MyClass);     // shared MyClass
var a3 = factory(unmanaged MyClass);  // unmanaged MyClass
var a4 = factory(borrowed MyClass);   // owned MyClass**
var a5 = factory(MyClass);            // owned MyClass**

// From D3, a4 and a5 returns "new borrowed MyClass"
//      which translates to "new MyClass" (direct equivalence)
//      which translates to "new owned MyClass" (default undecorated new)

class CompositeClass {
  var b1: owned MyClass;
  var b2: shared MyClass;
  var b3: unmanaged MyClass;
  var b4: borrowed MyClass;
  var b5: MyClass;            // borrowed MyClass**
}

proc concretefn(arg: owned MyClass) {}
proc concretefn(arg: shared MyClass) {}
proc concretefn(arg: unmanaged MyClass) {}
proc concretefn(arg: borrowed MyClass) {}
proc concretefn(arg: MyClass) {}            // borrowed MyClass

Undecorated MyClass is always syntactically equivalent to borrowed MyClass. No chance for any confusion because the rule any user would need to remember is that the two are always syntactically equivalent.

The main downside is that I marked the lines with double-asterisks that I think have "surprising" behavior for those that do not always remember the rule.

Make undecorated class illegal

var x1 = new owned MyClass();      // owned MyClass
var x2 = new shared MyClass();     // shared MyClass
var x3 = new unmanaged MyClass();  // unmanaged MyClass
var x4 = new borrowed MyClass();   // Error. Illegal. Cannot 'new borrowed'.
var x5 = new MyClass();            // Error: Undecorated class needs managed intent.

var y1: MyClass = new owned MyClass();      // Error: Undecorated class needs managed intent.
var y2: MyClass = new shared MyClass();     // Error: Undecorated class needs managed intent.
var y3: MyClass = new unmanaged MyClass();  // Error: Undecorated class needs managed intent.
var y4: MyClass = new borrowed MyClass();   // Error1: Undecorated class needs managed intent.
                                            // Error2: Illegal. Cannot 'new borrowed'.
var y5: MyClass = new MyClass();            // Error: Undecorated class needs managed intent.

proc factory(type t) {
  return new t();
}

var a1 = factory(owned MyClass);      // owned MyClass
var a2 = factory(shared MyClass);     // shared MyClass
var a3 = factory(unmanaged MyClass);  // unmanaged MyClass
var a4 = factory(borrowed MyClass);   // Error. Illegal. Cannot 'new borrowed'.
var a5 = factory(MyClass);            // Error: Undecorated class needs managed intent.

class CompositeClass {
  var b1: owned MyClass;
  var b2: shared MyClass;
  var b3: unmanaged MyClass;
  var b4: borrowed MyClass;
  var b5: MyClass;            // Error: Undecorated class needs managed intent.
}

proc concretefn(arg: owned MyClass) {}
proc concretefn(arg: shared MyClass) {}
proc concretefn(arg: unmanaged MyClass) {}
proc concretefn(arg: borrowed MyClass) {}
proc concretefn(arg: MyClass) {}            // Error: Undecorated class needs mangaed intent.

This is the conservative approach. We should start here because we can always lift any restrictions later. Just force the user to decorate everything. The main advantage is that we can glean how people are actually decorating their large-scale codes and then make a determination as to whether any defaults should be chosen, if any.

Also, I removed new borrowed to be more restrictive. If users really want new borrowed, a default can be set later.

Make some usage of undecorated class illegal

Compromise solution between the last two where we forbid it in any confusing cases.

var x1 = new owned MyClass();      // owned MyClass
var x2 = new shared MyClass();     // shared MyClass
var x3 = new unmanaged MyClass();  // unmanaged MyClass
var x4 = new borrowed MyClass();   // Error. Illegal. Cannot 'new borrowed'.
var x5 = new MyClass();            // owned MyClass (default undecorated new) // <--- Relaxed

var y1: MyClass = new owned MyClass();      // borrowed MyClass (from owned MyClass)     // <--- Relaxed
var y2: MyClass = new shared MyClass();     // borrowed MyClass (from shared MyClass)    // <--- Relaxed
var y3: MyClass = new unmanaged MyClass();  // borrowed MyClass (from unmanaged MyClass) // <--- Relaxed
var y4: MyClass = new borrowed MyClass();   // Error. Illegal. Cannot 'new borrowed'.
var y5: MyClass = new MyClass();            // borrowed MyClass (from owned MyClass)     // <--- Relaxed

proc factory(type t) {
  return new t();
}

var a1 = factory(owned MyClass);      // owned MyClass
var a2 = factory(shared MyClass);     // shared MyClass
var a3 = factory(unmanaged MyClass);  // unmanaged MyClass
var a4 = factory(borrowed MyClass);   // Error. Illegal. Cannot 'new borrowed'.
var a5 = factory(MyClass);            // Error: Undecorated class needs managed intent.

class CompositeClass {
  var b1: owned MyClass;
  var b2: shared MyClass;
  var b3: unmanaged MyClass;
  var b4: borrowed MyClass;
  var b5: MyClass;            // Error: Undecorated class needs managed intent.
}

proc concretefn(arg: owned MyClass) {}
proc concretefn(arg: shared MyClass) {}
proc concretefn(arg: unmanaged MyClass) {}
proc concretefn(arg: borrowed MyClass) {}
proc concretefn(arg: MyClass) {}            // borrowed MyClass // <--- Relaxed

Starting from the most conservative approach, we relax several restrictions by applying D1 and D2.

Preferences

My preferences would be:

  1. Make undecorated class illegal. Most conservative for now. More experience needed (like recent change of undecorated new from scoped new borrowed to new owned). We get forward progress by punting on the default behaviors for now. (Separately, we could add default undecorated new == new owned independent of this issue, but it's hard to not conflate the two when writing examples.)
  2. Make some usage of undecorated class illegal. Make the most confusing cases illegal.
  3. undecorated MyClass is always a borrow. Least preferred only because some behaviors could be considered surprising and I would rather users explicitly annotate those cases.
  4. Anything else.
  5. Anything else based around MyClass being generic. A generic MyClass just seems rife with confusion as demonstrated here.

Edit: Added functions that take a class argument. More text for pros/cons. Edit2: Final. Gah. Editing code in GitHub is hard.

BryantLam commented 5 years ago

One more based on make undecorated class illegal but relaxing only some cases of where MyClass === borrowed MyClass and NOT defining default undecorated new. I'd rank this proposal around 2.5.

var x1 = new owned MyClass();      // owned MyClass
var x2 = new shared MyClass();     // shared MyClass
var x3 = new unmanaged MyClass();  // unmanaged MyClass
var x4 = new borrowed MyClass();   // Error. Illegal. Cannot 'new borrowed'.
var x5 = new MyClass();            // Error: new class needs managed intent.

var y1: MyClass = new owned MyClass();      // borrowed MyClass (from owned MyClass)     // <--- Relaxed
var y2: MyClass = new shared MyClass();     // borrowed MyClass (from shared MyClass)    // <--- Relaxed
var y3: MyClass = new unmanaged MyClass();  // borrowed MyClass (from unmanaged MyClass) // <--- Relaxed
var y4: MyClass = new borrowed MyClass();   // Error. Illegal. Cannot 'new borrowed'.
var y5: MyClass = new MyClass();            // Error: new class needs managed intent.

proc factory(type t) {
  return new t();
}

var a1 = factory(owned MyClass);      // owned MyClass
var a2 = factory(shared MyClass);     // shared MyClass
var a3 = factory(unmanaged MyClass);  // unmanaged MyClass
var a4 = factory(borrowed MyClass);   // Error. Illegal. Cannot 'new borrowed'.
var a5 = factory(MyClass);            // Error: Undecorated class needs managed intent.

class CompositeClass {
  var b1: owned MyClass;
  var b2: shared MyClass;
  var b3: unmanaged MyClass;
  var b4: borrowed MyClass;
  var b5: MyClass;            // Error: Undecorated class needs managed intent.
}

proc concretefn(arg: owned MyClass) {}
proc concretefn(arg: shared MyClass) {}
proc concretefn(arg: unmanaged MyClass) {}
proc concretefn(arg: borrowed MyClass) {}
proc concretefn(arg: MyClass) {}            // borrowed MyClass // <--- Relaxed
mppf commented 5 years ago

I'm relatively happy with @BryantLam's suggestions 1-3. I'd probably personally go for something like the 2.5 above. I think the rule about having to decorate class types passed to a type alias ought to address some of the concerns with having to decorate field types in a generic context. (In particular, you wouldn't have to decorate a generic field like type t; var someField: t, because the rule about needing to decorate the class type on passing it to a type argument would take care of it).

bradcray commented 5 years ago

I've been mulling over this issue while taking a break from it, and dove back into it today. I've drafted two variants of Bryant's list of options, one of which could be described as "undecorated class types mean owned for new variables and borrowed for existing variables"; the other of which is another stab at the "undecorated class types are generic" theme, which still seems very intuitive to me if we can pull it off. I'm going to iterate on these with Michael offline before posting them here because I suspect he'll either find flaws in them or help me make them better before I start wasting others' time with rough drafts.

In the meanwhile, here are my thoughts on Bryant's proposals:

BryantLam commented 5 years ago
  • I think that whether new borrowed remains legal is orthogonal to this conversation and generally a distraction

Agreed. I included new borrowed for some notion of completeness. It deserves its own issue, likely affected by the outcome of this issue.

  • I don't think Bryant's D3 follows, for me at least. Specifically, once we no longer interpret 'new MyClass' as 'new borrowed MyClass', ...

Agreed. D1 and D2 generate D3. If you don't accept D1 or D2, then D3 doesn't make sense.

  • It seems odd to me to make 'new C()` illegal ...

I included it mostly just for the conservative case (i.e., no-defaults approach) as a starting point with rank 1. I like the new C() as new owned C() (D1) behavior and implied that it was the most straightforward proposal to relax; the resulting combination would be equally of rank 1. But it depends on whether Chapel does attempt MyClass as an undecorated generic type, which is partly why the no-defaults approach ended up rank 1.

bradcray commented 5 years ago

I've drafted two variants of Bryant's list of options, one of which could be described as "undecorated class types mean owned for new variables and borrowed for existing variables"; the other of which is another stab at the "undecorated class types are generic" theme, which still seems very intuitive to me if we can pull it off.

In the end, I abandoned the first of these two proposals (it didn't hold up for me) and focused more on the second. Here's what I've come up with and am curious for people to poke holes in (specifically, @BryantLam, I've had trouble recreating what problem you got wrapped around when taking a stab at a generic interpretation above, so am not sure whether this proposal suffers from it as well).

Concept: MyClass is generic

Theme: If I write MyClass in a type constraint situation, I'm really not saying much beyond "this is some flavor of MyClass", without specifying which one; so let's interpret it as "any flavor."

Additional choices:

Alternatives left open in this discussion:

1) Handling of undecorated MyClass:

a) Alternatively, we could disallow new MyClass() like we'd disallow any other new of a partially specified/instantiated generic type.

b) Or, we could have 'MyClass' be interpreted as 'owned MyClass' in cases where it is underconstrained other than just new MyClass() (e.g., variable declarations like y6 below).

2) For handling of default and const intents in [partially] generic contexts, two approaches are considered:

a) "similar to master": This approach treats these cases similar to master today by converting types to borrows in order to (i) support a common case, (ii) avoid performance overhead for passing owned/shared to generics, (iii) make use of lifetime checking in such cases.

b) "treat like any other generic": This approach continues with the theme of "what if this were just like any other generic?" and tries to come up with an answer that preserves type (i.e., given proc foo(x) { ... } and foo(actual), x will have the same type as actual) to explore what the impact of treating class ownership slightly less specially would be.

var x1 = new owned MyClass();      // owned MyClass
var x2 = new shared MyClass();     // shared MyClass
var x3 = new unmanaged MyClass();  // unmanaged MyClass
var x5 = new MyClass();            // owned MyClass (or illegal, based on 1a)

var y1: MyClass = new owned MyClass();      // owned MyClass
var y2: MyClass = new shared MyClass();     // shared MyClass
var y3: MyClass = new unmanaged MyClass();  // unmanaged MyClass
var y5: MyClass = new MyClass();            // owned MyClass (or illegal, based on 1a)

var y6: MyClass;  // error: Not a fully-concrete type
                  // (or: `MyClass` expands to `owned MyClass`, based on variation 1b)

[const] ref ra: MyClass = myOwnedClass;             // refers to owned MyClass
[const] ref ra: MyClass = myOwnedClass.borrow();    // refers to borrowed MyClass
[const] ref r1: MyClass = new owned MyClass();      // refers to owned MyClass
[const] ref r2: MyClass = new shared MyClass();     // refers to shared MyClass
[const] ref r3: MyClass = new unmanaged MyClass();  // refers to unmanaged MyClass
[const] ref r5: MyClass = new MyClass();            // refers to owned MyClass (or illegal, based on variation 1a)

proc factory(type t) {
  return new t();
}

var a1 = factory(owned MyClass);      // owned MyClass
var a2 = factory(shared MyClass);     // shared MyClass
var a3 = factory(unmanaged MyClass);  // unmanaged MyClass
var a5 = factory(MyClass);            // owned MyClass (or illegal, based on variation 1a)

class CompositeClass {
  var b1: owned MyClass;
  var b2: shared MyClass;
  var b3: unmanaged MyClass;
  var b4: borrowed MyClass;
  var b5: MyClass;            // depends on how b5 is initialized
                              // e.g., init() says this.b5 = new owned MyClass();   => owned MyClass
                              //                   this.b5 = myOwnedClass;          => owned MyClass
                              //                   this.b5 = myOwnedClass.borrow(); => borrowed MyClass
                              // in the base proposal, such initialization would be required since 'MyClass' is not a complete type
                              // in variant 1b, it would become an 'owned MyClass' if left uninitialized
}

proc concretefn(arg: owned MyClass) {}      // owned MyClass
proc concretefn(arg: shared MyClass) {}     // shared MyClass
proc concretefn(arg: unmanaged MyClass) {}  // unmanaged MyClass
proc concretefn(arg: borrowed MyClass) {}   // borrowed MyClass

// Given these calls...
//
semiconcretefn(new <flavor> MyClass());
semiconcretefn(my<flavor>MyClass);

// Interpret these procedures...
//
proc semiconcretefn([const] in[out] arg: MyClass) {}   // `arg` is a `<flavor> MyClass`
                                                       // similar to `[const|var] arg: <flavor> MyClass = actual;`
proc semiconcretefn(out arg: MyClass) {}               // `arg` is a `<flavor> MyClass`
                                                       // similar to `var arg: <flavor> MyClass;` if/when legal
proc semiconcretefn([const] ref arg: MyClass) {}       // `arg` is a `<flavor> MyClass`
                                                       // similar to `[const] ref arg: <flavor> MyClass = actual;`?

// Variant 2a interpretations start here:
//
proc semiconcretefn([const] ref arg: MyClass) {}       // `arg` is a `borrowed MyClass`
                                                       // treated like `const in arg: borrowed MyClass = actual;` (relies on coerce-to-borrow)
proc semiconcretefn(arg: MyClass) {}                   // same as above

// Variant 2b interpretations start here:
//
proc semiconcretefn([const] arg: MyClass) {}           // `arg` is a `<flavor> MyClass`...
                                                       //   ...passed by 'const in' for <flavor>==`unmanaged` or `borrowed`
                                                       //   ...passed by `const ref` for <flavor>==`owned` or `shared`
proc semiconcretefn(arg: MyClass) {}                   // same as the preceding

// Given these calls...
genericfn(new <flavor> MyClass());
semiconcretefn(my<flavor>MyClass);

// Interpret these procedures...
proc genericfn([const] in[out] arg) {}   // `arg` is a `<flavor> MyClass`
                                         // similar to `[const|var] arg = actual;`
proc genericfn(out arg) {}               // `arg` is a `<flavor> MyClass`
                                         // similar to `var arg: actual.type;` ?
proc genericfn([const] ref arg) {}       // `arg` is a `<flavor> MyClass`
                                         // should be similar to `[const] ref arg = actual;`

// Variant 2a interpretations start here:
//
proc genericfn([const] arg) {}           // `arg` is a `borrowed MyClass`
                                         // treated like `const in arg: borrowed MyClass = actual;` (relies on coerce-to-borrow)
proc genericfn(arg) {}                   // same as preceding

// Variant 2b interpretations start here:
//
proc genericfn([const] arg) {}           // `arg` is a `<flavor> MyClass` (treated like `const ref` above)
                                         //   ...passed by 'const in' for <flavor>==`unmanaged` or `borrowed`
                                         //   ...passed by `const ref` for <flavor>==`owned` or `shared`
proc genericfn(arg) {}                   // same as preceding
mppf commented 5 years ago

I've historically been very concerned with whether or not the potential for ownership transfer is clear in function signatures / chpldoc API listings.

To some degree, 2a in the above proposal comes from this viewpoint. In particular, I think that ownership transfer should not occur when passing an owned MyClass actual into proc f(arg: MyClass).

The thing is, I think that 2b in the above proposal also has this property. In that setting, proc g(arg: owned MyClass) would not create ownership transfer by itself. (The default intent for owned would change to const ref - it is currently in on master). Instead, you'd have to use the in intent to indicate ownership transfer. The nice thing about using the in intent for this is that it applies equally well to generic cases, such as proc gg(arg) or proc f(arg: MyClass) (where f accepts MyClass with any sort of memory management). Neither of these cause ownership transfer, but if you put in intent, then it does transfer ownership for owned actuals and increments reference counts for shared actuals (and does nothing different for borrowed or unmanaged actuals).

This is also appealing to me also because it matches when a copy would occur with a by-value record class.

So, I'm saying that this API property - the API shows whether ownership transfer is possible - is available even if we consider proc f(arg: MyClass) to be generic w.r.t. management style (i.e. option 2b).

That leaves

(ii) avoid performance overhead for passing owned/shared to generics,

As my remaining concern about 2b. I think it might be better to view that as an optimization problem rather than a language design one.

mppf commented 5 years ago

Also, I'm not sure if this is obvious to everyone reading, making proc f(arg: MyClass) generic w.r.t management style would imply two other changes:

The second change in particular seems like a nice simplification (because it removes a special rule).

BryantLam commented 5 years ago

I agree with everything in https://github.com/chapel-lang/chapel/issues/12917#issuecomment-499233360 except this statement:

@mppf: As my remaining concern about 2b. I think it might be better to view that as an optimization problem rather than a language design one.

Taken pedantically, language design is about making one set of optimization choices (language) to make another set of choices (impact on compiler and on users) easier or harder.

For the function signatures and variants 2a and 2b: The const ref generic approach is appealing because it makes the compiler's implementation simpler and more consistent with the rest of the language design, but there's a concern with code bloat regarding 2b with increased use of generics. If this concern is resolved, then the 2b approach is acceptable to me, though I suspect the resolution will involve non-trivial compiler optimization, arguably negating the benefits of the simpler implementation. That's why I think 2a should be the default as it's an easier solution to comprehend and implement, though there is some benefit to passing in user-defined managed types with queryable const functions (e.g., check the ref count?) if such behavior was desired.

Specifically:

proc fn(arg: MyClass) { ... }
proc main() {
  fn(new owned MyClass())
  fn(new shared MyClass())
  fn(new unmanaged MyClass())
  fn(<borrowed MyClass>)
}

// Variant 2a --->
proc fn(arg: borrowed MyClass) { ... }

// Variant 2b --->
proc fn(const ref arg: owned MyClass) { ... }
proc fn(const ref arg: shared MyClass) { ... }
proc fn(const in arg: unmanaged MyClass) { ... }
proc fn(const in arg: borrowed MyClass) { ... }

What optimization strategies would prevent the code bloat in 2b?

Is it worth the consistency?

How do these optimization strategies fit in with user-defined managed types?

Arguably, one kind of optimization would apply in the backend so the frontend could still do the typical incorrect-usage-due-to-const detection if a user ran .release() on the const-ref managed type, but then then arg would lower into a borrow-equivalent to minimize the number of pointer dereferences and code bloat. For example, the memory layout of managed types / smart pointers might need to be defined in a way that it would be trivial to cast to a borrow. (Is that layout enforceable for user-defined managed types?)

mppf commented 5 years ago

@BryantLam - I think your are saying that you are concerned about the impact on the size of the generated code. Is that accurate? (Or, are you concerned about compile time, or about program performance?)

In my earlier comments, I was talking about the impact on program performance.

Generally speaking the amount of code for a generic is limited by the usage at the call sites. In particular it's likely that fn in your example above will only be instantiated for one or two of the variants in practice. The compiler does not instantiate generics that aren't applicable (and it will only resolve the bodies of generic instantiations that are actually called).

BryantLam commented 5 years ago

I think your are saying that you are concerned about the impact on the size of the generated code. Is that accurate? (Or, are you concerned about compile time, or about program performance?)

All of the above.

I recognize that generics are only instantiated for the types that use them, but I think a feature as systemic as the managed types will yield a lot of extra code and more compile time without strong consideration for reducing the code bloat. Maybe it's a bit of premature optimization so perhaps it's likely not an issue, but I'd be interested in trial results on an existing substantial codebase to prove me wrong.

mppf commented 5 years ago

@BryantLam - I guess I am not convinced by that argument. In particular, I think there's another logical step to it that we havn't discussed yet... and that I'm not sure I agree with.

If a user wrote

proc g(arg) { ... }

then they have a generic function that could generate a large amount of code (or be involved in a lot of compile time). But the reason Chapel makes using generics easy is that it makes the job of programming easier, while still allowing good program performance. So this is already an intentional choice on the part of the language (as compared with, say, C++ with template void g<T>(T arg) { ... } which is not so easy to write).

If we chose 2b, we would be in effect saying that MyClass is a generic type. If a user doesn't want a generic type (which is reasonable in a variety of situations), then they would decorate it, e.g. proc fn(arg: borrowed MyClass).

It's already the case that proc f(arg:MyGenericRecord) or proc f(arg:MyGenericClass) are generic functions if those types are generic.

But even with 2a, we are thinking about having a way to say "the argument a MyClass but can accept any decorator". That is what the managed? in the original design did. So the issue is still present, isn't it, in the event users write managed? MyClass (or whatever the equivalent is) everywhere?

I think the missing logical step from your argument is something like "Users will use MyClass as an argument type because it is shorter/simpler/more intuitive and then be confused/surprised by the resulting code bloat".

However, I think users will get used to MyClass being generic - just as they are used to proc f(arg) being generic or proc g(arg:MyGenericClass) being generic. In fact they might find understanding when MyClass is generic easier to get used to with 2b (since MyClass will be generic in variable type declarations in 2a or 2b - but not in argument contexts in 2a).

From a language design viewpoint of "We need to be able to represent all of these things", I think 2b is definitely more appealing, because arg:MyClass would mean arbitrary management type, and we wouldn't need to have a way to write something like managed? MyClass... which seems to defy easy characterization into intuitive keywords because it adds more keywords and yet makes the argument less restrictive (i.e. more generic). I think that's counterintuitive strictly from a language usability viewpoint. But 2b neatly solves that issue.

Anyway, I think it comes down to deciding which of 2a and 2b is less confusing (for our various definitions of confusing).

BryantLam commented 5 years ago

I think there's another logical step to it that we havn't discussed yet... and that I'm not sure I agree with.

Which part is the part that you weren't sure you agreed with? I think it was something in your post, but I wasn't sure which.

Compelling argument. I reiterate that I have some concerns over performance optimizations within the compiler and user code, but I'm only neutral or weakly against 2b. If you're strongly for the 2b solution on the grounds of usability and consistency, I'm good to go.

I appreciate that it is a consistent solution that would also remedy the confusion I had in https://github.com/chapel-lang/chapel/issues/12917#issuecomment-488054548. (The code is wrong with 2b since I need to add in intent.)

mppf commented 5 years ago

Which part is the part that you weren't sure you agreed with? I think it was something in your post, but I wasn't sure which.

Earlier you had said:

I think a feature as systemic as the managed types will yield a lot of extra code and more compile time without strong consideration for reducing the code bloat

My thinking was that this argument unpacks into two parts:

  1. Users will use MyClass as an argument type without understanding that it's actually generic
  2. Users will then be surprised by the resulting code bloat

It's the first of these that I don't think I agree with. (I think users will understand when they are writing generic code and when they aren't).

Compelling argument. I reiterate that I have some concerns over performance optimizations within the compiler and user code, but I'm only neutral or weakly against 2b. If you're strongly for the 2b solution on the grounds of usability and consistency, I'm good to go.

I can see advantages to either one. I worry that with 2a users will not be able to remember the different meanings of MyClass in different contexts. Also, in practice, I've run into cases where I've been surprised/confused by the untyped-actual-becomes-borrow rule. I think that will be harder to address these potential confusions than to improve compiler or program performance. Improving compiler or program performance can plausibly be handled in the implementation without language changes at all.

Also, for the program performance issue, we could consider a mostly unrelated language change to improve optimization. We could say that a function accepting a const ref owned (or other record) can read the fields of the record at the start of the function and re-use those. (That might be a great idea or a horrible one, but if we want to discuss it further it should get its own issue. I'm just bringing it up as a for-instance).

I had originally been in favor of 2a but I think now I'm leaning towards 2b.

vasslitvinov commented 5 years ago

I'd like to draw on our experience with OOP as Chapel compiler writers.

I think of our program representation as a tree of Expr classes, or "nodes", with some extras:

I would like to represent the parent-to-child links as owned fields. Ideally, the other fields would be borrowed. However it would be very hard to prove to the lifetime checker that this is correct. Somehow it seems that shared is not appropriate either, perhaps because of the undesired reference-counting overhead. So in practice those fields would be unmanaged.

class Expr: BaseAST {
  unmanaged parentExpr: Expr?;
    // a child-to-parent link
    // nil when this node is not inTree()
  unmanaged prev, next: Expr?;
    // sibling-to-sibling links
    // nil when this node is not on a list
  unmanaged parentSymbol: Symbol?;
    // a link to outside the tree
    // nil when this node is not inTree()
}

class CondStmt: Expr {
  owned condExpr, thenStmt: Expr;
  owned elseStmt: Expr?;
    // parent-to-child links
    // the else-part may be absent
}

class SymExpr: Expr {
  unmanaged Symbol sym;
    // a link to outside the tree
    // multiple SymExprs and a DefExpr can reference the same Symbol
    // maybe all references to a Symbol can be "shared" ?
    // moot point for some global Symbols ex. rootModule or dtVoid.symbol
}

class AList {
  // an AList is owned by a parent Expr and contains links to its children
  // whilch are always non-nil
}

Since the compiler does its own memory management for AST classes, the above fields are de facto unmanaged, even for the parent-to-child-link fields.

I suspect that the compiler in some cases stores nil in condExpr, thenStmt, and sym fields, so we may not be able to declare them non-nilable. Aha, one example is param-folding of a conditional, which we do like so: currCondStmt.replace(currCondStmt.thenStmt.remove()).

During parsing, the AST is built bottom-up:

proc buildBlock(in expr: Expr) {
  // I am using 'in' to mean "take over the ownership"
  return new owned BlockStmt(expr);
   // pass ownership of 'expr' to the new object
}

proc buildCondStmt(in condExpr, in thenStmt, in elseStmt) {
  return new owned CondStmt(condExpr,
                            buildBlock(thenStmt),
                buildBlock(elseStmt));
    // pass ownership of the subtrees to the new object
}

// etc.

A compiler analysis without transformation obviously "borrows" existing nodes, which may be nilable or not:

proc toSymExpr(arg): SymExpr? return (arg: SymExpr);
  // the argument is a nilable borrowed Expr
  // returns a nilable borrowed SymExpr

const firstArg: SymExpr? = toSymExpr(myCallExpr.get(1));
  // a borrow
....

if (secondArg: SymExpr = toSymExpr(....)) {
  // within this block, 'secondArg' is a non-nilable borrow const
}

Adding transformations...

proc borrowed Expr!.insertAfter(in expr): void  { ..... }
  // It adds 'expr' as a child to some parent, so it better
  // take over the ownership of the argument.

proc owned Expr!.remove(): owned Expr!
{ ...; return this; }
  // It removes 'this' as a child from some parent.
  // We want it to return the child as "owned"
  // because we may want to call 'insertAfter' on that node.

proc owned Expr!.replace(in expr): owned Expr! {
  this.insertAfter(expr);
  return this.remove();
}

One occasionally-non-trivial thing for me is know whether the AST node held in a particular variable is inTree().

An example is origSE and iterCall locals/formals in resolveForallHeader() and the functions that it invokes, ex. buildForallParIterCall().

It would be nice if I can use the owned/borrowed attribute of the variable's type to tell one way or the other. The challenge is when the node referenced by a variable is originally inTree(), then it is removed and perhaps reinserted later, conditionally or always. To remove it, I need it owned. To have it owned before/without removal, I probably have to have a ref to it. So the owned/borrowed distinction does not correlate to inTree()/not.

var expr = findDefinition();      // should 'expr' be owned? borrowed? ref?
if (needToRemoveDefinition(expr)) // 'expr' is still inTree() here
{
  expr->remove();
  if (needToReinsert(expr))       // 'expr' is not inTree() here
    otherExpr->insertAfter(expr);

  return expr;                    // owned? borrowed?
}

Other than the challenge above, I am thinking that:

Regarding code bloat due to MyClass being generic over memory management -- seems this wouldn't be a problem here. Because most functions in our compiler have only one or a couple of call sites, statically. The functions invoked many times are either small or require particular discipline for their arguments, so would be instantiated only in a couple of different ways.

vasslitvinov commented 5 years ago

The idea of MyClass being generic over memory management appeals to me because then we can make it consistently generic in several possible ways:

The tricky part that I see in the above discussion is the meaning of the default/const intent for arg:MyClass and for a fully-generic arg. The philosophy currently implemented on master is that (a) this is what the user would like to write in the common case and (b) it should mean "the formal borrows from the actual". Sounds like we are still standing by this philosophy? Both 2a and 2b offer this. Considering that in 2b, borrowing is achieved by passing an owned by const ref.

While 2a achieves this philosophy better, I am very uneasy about its having MyClass mean different things for variable/field vs. formal.

For 2b, I am concerned about the performance of passing owned by const ref. And also about the semantics -- this kind of "borrow" may disappear if the owning actual releases its ownership while the callee is running. There is even no argument intent that by itself allowing borrowing "the efficient way". The user would have to say borrowed explicitly to get that.

mppf commented 5 years ago

@vasslitvinov - over on #13161 about nilability of MyClass, we are leaning against making it generic. Does that change your answer here?(or do you want to comment there?)

mppf commented 5 years ago

Supposing MyClass has "generic management".

In a case like this:

proc f(type t) { }
class MyClass { }
f(MyClass);

This apparently passes something generic (MyClass with generic memory management) to a type argument. That's not allowed today (but I think it's reasonable to imagine it will work in the future). Should this be an error today or should it "decay" to the borrow type?

bradcray commented 5 years ago

I'd lean towards having this case be treated as an error today. If there were compelling practical use cases that suggested otherwise, I'd reconsider. This seems vaguely related to some questions @benharsh has been wrestling with recently regarding passing partially instantiated types to functions like this (where we also discussed not supporting that for now).

mppf commented 5 years ago

@bradcray - my prototype is currently running into this case when trying to type construct owned MyClass but I can fix that another way. Thanks for the quick viewpoint!

mppf commented 5 years ago

I think that the undecorated-is-generic proposal also implies that this inside of a class method has generic type (generic management). This is similar to what we wanted for type methods. Is this expected, or is it suprising?

mppf commented 5 years ago

I suppose that might present problems with virtual method dispatch on owned... (Edit: in particular, I think we'd need to instantiate potential virtual child methods with all of the memory management styles and then have separate tables for these... This seems like a pretty severe problem).

(Edit 2: I'm concerned that this would add confusion because we'd now have override+overload by default and users have historically found that combination very confusing. I'm also concerned it would add unnecessary complexity to the single-inheritance classes idea.)

mppf commented 5 years ago

If we have a type method, like this

class MyClass {
  proc type foo() {  }
}

MyClass.foo();

then technically within the compiler the MyClass becomes an actual argument for a type formal and so we end up in a case like in the above comment - when MyClass is considered to have generic management, this becomes an error.

Should users have to write borrowed MyClass.foo() (which doesn't parse today) or should we make such type methods default to using the borrowed type i.e. borrowed MyClass ?

mppf commented 5 years ago

Two notes from the implementation effort:

  1. Unit test prototype code uses lambdas to capture functions like proc test(arg: Test). But these are now generic and need to be written proc test(arg: borrowed Test). Probably possible to sort out in the compiler support for unit testing.
  2. Certain == operations between class that tests were doing now have unclear behavior. For example what should MyClass == borrowed MyClass do?
bradcray commented 5 years ago

For point 2, my sense is that this should either return false or potentially just not resolve (e.g., can't compare concrete and generic types). For example given a generic record type R, it seems like we wouldn't want R == R(int) to work...? (@benharsh?)

benharsh commented 5 years ago

My intuition is to allow comparisons between concrete and generic types that will return false for a case like R == R(int).

mppf commented 5 years ago

@vasslitvinov wrote

The idea of MyClass being generic over memory management appeals to me because then we can make it consistently generic in several possible ways:

  • over its generic fields, if any
  • over memory management
  • over nilability (#13161)

I'm worried that it MyClass being generic-management but non-nilable (ie not generic nilability) will be confusing, especially since borrowed etc itself is generic-nilability - e.g. proc f(arg: borrowed) accepts nilable and non-nilable. See also https://github.com/chapel-lang/chapel/issues/13161#issuecomment-511412698.