nil-checking and conditionals

[mppf]

On the question of if let vs. inferring non-nilness based upon conditionals, we have discussed this a bit already #12614 (comment) #12614 (comment). In this issue I'd like to focus more on that question specifically.

Here is the example we will discuss, starting with :

     var current: borrowed MyClass? = ...;
     if current != nil {
       fnExpectingNotNil(current!);  // the `!` is currently required
       genericFn(current);
     }

     proc fnExpectingNotNil(arg: borrowed MyClass) { }
     proc genericFn(arg) { }

I know of 4 strategies for improvement within this kind of conditional pattern:

1a. If-let, following Swift
E.g.

     if let c = current {
       fnExpectingNotNil(c);
       genericFn(c);
     }

Pros:

• precedent in Swift
• low potential confusion about modifying c vs current
• low potential confusion about changing type of c vs current
• genericFn does not require further handling of nilability of the argument

Cons:

• extra typing
• let syntax was on the way out in Chapel...

1b. If-var variant (from #12614 (comment))
E.g.

     if var c = current {
       fnExpectingNotNil(c); 
       genericFn(c);
     }

Pros:

• fairly similar to Swift
• low potential confusion about modifying c vs current
• low potential confusion about changing type of c vs current
• genericFn does not require further handling of nilability of the argument

Cons:

• extra typing

2a. inference-based with shadow variable

     if current != nil {
       // within the conditional, current.type is non-nilable borrowed MyClass
       fnExpectingNotNil(current); // OK
       genericFn(current);
     }

Pros:

• less typing
• non-nilness of current applies to calls to any-t
• genericFn does not require further handling of nilability of the argument

Cons:

• potential confusion about modifying c vs current
• potential confusion about changing type of c vs current

2b. inference-based with allowed coercions

     if current != nil {
       // within the conditional, current can coerce from MyClass? to MyClass,
       // but current.type is still borrowed MyClass?
       fnExpectingNotNil(current); // OK due to coercion
       genericFn(current);
     }

Pros:

• less typing

Cons:

• passing current.type somewhere will give MyClass? but we know it's not nil here
• genericFn needs further handling of nilability of the argument. non-nilness doesn't apply to generic functions called and passed current (unless analysis becomes interprocedural)
• when this applies might depend on quality of compiler analysis

[mppf]

In a separate (off-line) discussion, @bradcray indicated a preference for something along the lines of 1a/1b.

[mppf]

Note that in 1b, I would not expect to allow a ref or const ref variant for the same reason described in #13621 (comment) :

     if const ref c = current {
       current = nil;
       fnExpectingNotNil(c);  // error that isn't checked at runtime or compile-time!
     }

[bradcray]

I kinda like 1b, and kinda don't. I like that it tries to compose existing concepts (introduction of a new symbol via var) to avoid introducing new ones. But it also makes me wonder if we should permit other variable declaration forms in conditions (similar to C's if (MyClass* c = ...) { ...) to make it more orthogonal. And if we did that, would we be adding value or clutter to the language?

Note that in 1b, I would not expect to allow a ref or const ref variant

What about a const variant?

[mppf]

I kinda like 1b, and kinda don't. I like that it tries to compose existing concepts (introduction of a new symbol via var) to avoid introducing new ones. But it also makes me wonder if we should permit other variable declaration forms in conditions (similar to C's if (MyClass* c = ...) { ...) to make it more orthogonal. And if we did that, would we be adding value or clutter to the language?

Actually I don't think this kind of code has much use outside of checking for not-nil.
It is true that you can currently write Chapel code like if 0 then ... and so we could generalize to if var = someFunctionReturningInt() but this if 0 idiom does not seem to come up much in practice. Nonetheless I don't see a problem generalizing this beyond classes to anything that can go in an if. I think if we found this unreasonable, I'd rather disallow if 0 than avoid the generalization.

Note that in 1b, I would not expect to allow a ref or const ref variant

What about a const variant?

a const variant would be fine

[bradcray]

Actually I don't think this kind of code has much use outside of checking for not-nil.

That's a good point. I think you're arguably saying that the patterns I might write in C using the feature are more or less what we're trying to support here anyway (?).

I think this makes me tend to like 1b pretty well. 1a's of course not bad either depending on how we feel about let overall (I see Louis wanting more of it over on chat today). Then a question becomes for me: if the language preserved let, which of the two would be the better match?

[mppf]

That's a good point. I think you're arguably saying that the patterns I might write in C using the feature are more or less what we're trying to support here anyway (?).

Yes, that's what I'm saying. Or, at least that's the case in the compiler, where we write things like if (SymExpr* se = toSymExpr(something)) { ... } all over the place. These are working with class casts, which in Chapel we'd write as if const se = something: SymExpr?.

I think this makes me tend to like 1b pretty well. 1a's of course not bad either depending on how we feel about let overall (I see Louis wanting more of it over on chat today). Then a question becomes for me: if the language preserved let, which of the two would be the better match?

I'm not personally that in to let - it seems like something that is easily expressed with the other language elements. If we were keeping let then I think the main question I'd have is if the var vs const distinction is useful in this context.

[vasslitvinov]

I'd like to mention an example from our Futures module:

    inline proc isValid(): bool {
      return ((classRef != nil) && classRef.valid);
    }

where classRef is a field of the Future record for which the isValid is a method. Sounds like the method author is assuming that classRef can't be modified concurrently with calling isValid(). I think this is similar to the if (SymExpr* se = toSymExpr(something)) pattern discussed above.

[vasslitvinov]

I propose to go forward with 1a and/or 1b:

if [let|const|var] c = EXPR {
  // 'c' is non-nilable here
  // the 'var' form allows assigning non-nilable values to 'c'
}

// allow this in while-loops, too
while [let|const|var] c = EXPR {
  .........
}

I do not have a preference for which of let, const, var we will allow.

My preference for this form is based on its robustness and clarity:

• It is safe w.r.t. concurrent updates to EXPR that could make it nil.
  • I suggest not allowing ref or const ref forms because they are not safe.
  • Ensuring safety with a compiler analysis would be hard, ex. when EXPR is a field access:

if myC.someField != nil {
  myFunction();          // did myFunction set someField on the myC instance?
  myC2.someField = nil;  // does myC == myC2 or not?
  myC.someField.someMethod();   // should the compiler allow this?
}
proc myFunction() {
  myArray[idx].someField = nil;
}

• We do not need a subtlety / new typechecking/resolution rule to have the same expression behave differently within such a conditional, ex. giving myC.someField a different type under if myC.someField.

One example where this form shines is in linked-list processing:

var current = mylist.head;
while const c = current {
  .......
  current = c.next;  // this would be hard to make legal under 2a/b
}

[mppf]

if var c = EXPR and if const c = EXPR are more appealing to me than let. (As I've said before). Does anybody want to argue for why they think let would be better here?

[bradcray]

I talked to some users who also preferred the 1a/1b options over the 2a/2b options (without any strong preference between the options).

[vasslitvinov]

An interesting aspect of 1a/1b is what memory management the let-variable has. In this code:

if const c = EXPR {
  // then-block
}

we can give c the same memory management as EXPR if it is shared, borrowed, or unmanaged.

However, if EXPR is owned, then it becomes challenging. We could "steal" the class from EXPR for the duration of the then-block, then return it back. However this works only when EXPR is a modifiable variable and even then has a few drawbacks.

PR #17047, which implements (1b), makes c a borrow -- unless EXPR is unmanaged, in which case c is also unmanaged.

[bradcray]

PR #17047, which implements (1b), makes c a borrow -- unless EXPR is unmanaged, in which case c is also unmanaged.

This matches where my thinking went upon reading about the owned issue, though I think I would've made c a borrow for unmanaged as well. Specifically, isn't the implication that each if const c = myUnmanagedClass conditional will require a delete within its body? That seems like it will become more annoying than useful, and I'd be inclined to just treat all cases equivalently to avoid the hassle. [edit: No Brad, we're not getting a new object due to this declaration, just a new borrowed or unmanaged reference to the object]

[bradcray]

Belatedly, I'm realizing that the behavior you've proposed and implemented matches what we'd do for normal variable initializations from de-nil'ed class expressions, since:

class C {
}

var myO = new owned C?();
var myS = new shared C?();
var myB = new borrowed C?();
var myU = new unmanaged C?();

writeln(myO.type:string);
writeln(myS.type:string);
writeln(myB.type:string);
writeln(myU.type:string);

const myO2 = myO!;
const myS2 = myS!;
const myB2 = myB!;
const myU2 = myU!;

writeln(myO2.type:string);
writeln(myS2.type:string);
writeln(myB2.type:string);
writeln(myU2.type:string);

results in:

owned C?
shared C?
borrowed C?
unmanaged C?
borrowed C
borrowed C
borrowed C
unmanaged C

This makes me even more confident that your implementation is correct. borrowed is correct for the cases you've implemented it for. It does make me wonder whether or not we've made the wrong decision for unmanaged in the general case. In a larger scope, I can see how inheriting the unmanaged behavior might seem reasonable; but in the smaller scope of this conditional form, it seems more annoying than helpful, given that the intention of the feature is productivity. So if we were to argue they should all be borrowed for orthogonality, it makes me wonder whether ! should return a borrowed for unmanaged in the general case as well.

Tagging @mppf for his experience and expertise with using and implementing nilable classes.

[vasslitvinov]

@bradcray indeed we should look at why we made a special case for unmanaged for !. @mppf do you know?

My initial implementation had everything be a borrow uniformly. Alas, then I could not make the changes that I temporarily reverted in 391973e. Here is the issue:

We use umanaged extensively in our array implementation. For example, BaseArr.prev and BaseArr.next are unmanaged. If let-var produced a borrow, we couldn't assign it to unmanaged when inserting into the linked list in _domain.add (showing only the essential part):

proc _domain.add_arr(x:unmanaged BaseArr) {
  if const ahead = _arrs_head {
    x.next = ahead;   // x.next is unmanaged, cannot assign a borrow into it
    ahead.prev = x;
  }
  _arrs_head = x;
}

[vasslitvinov]

To support the case where the programmer wants to retain the memory management being tested, I propose a variant on the let-var construct that specifies the type explicitly, for example:

if var myNonNilable: owned = someExpr() {
  // myNonNilable took over the ownership of the result of someExpr()
}

[bradcray]

I was going to propose the same, but had been waiting until we had a compelling use case.

[vasslitvinov]

(1b) is now implemented with #17047 and #17304.

This resolves the bulk of this issue, so I am closing it.

I have forked off the remaining items into follow-up issues:

• Lifetime of a 'const' borrow variable to ignore the variable's scope? #17317
• Extend control-flow declarations to preserve memory management? #17575
• Extend control-flow declarations to allow non-class types? #17576