Chapel: unable to resolve type on shared class with `proc !=` (RE: need compiler error for non-overloadable operators)

[BryantLam]

Summary of Problem

Compiler issue with 1.19.0. The following code only compiles if you omit the != function. This code should not compile because the == and != operators are not intended to be overloaded on classes.

class Foo {
  var next: shared Foo; // error: unable to resolve type
}

// Ok
proc ==(lhs: borrowed Foo, rhs: borrowed Foo): bool {
  return lhs.next == nil;
}

// Omit this function and it compiles.
proc !=(lhs: borrowed Foo, rhs: borrowed Foo): bool {
  return lhs.next != nil;
}

var x1 = new Foo();

Edit: Revised 2020-02-11

class Foo {
  var next: shared Foo?; // error: unable to resolve type
}

proc ==(lhs: borrowed Foo, rhs: borrowed Foo): bool {
  return lhs.next == nil;
}

proc !=(lhs: borrowed Foo, rhs: borrowed Foo): bool {
  return lhs.next != nil;
}

var x1 = new Foo();

This code still compiles as of chpl 1.21.0 pre-release circa 2020 January. There's debate about whether or not it should compile.

[mppf]

@BryantLam - I don't think we intend to support overloads for == or != on classes. AFAIK we decided not to support overloads for = or copy-initialization on them. You can do those things with records.

Nonetheless there could be a better error message. In this case, I suspect (but havn't started digging) that the compiler thinks that the call to != is recursive, which probably isn't what you intend (even if it did compile).

[bradcray]

I was going to answer similarly. It's been a longstanding grey area in the language whether or not you're allowed to define = on a class where the traditional thinking has been "you shouldn't be allowed to; it should always result in the pointer-to-class assignment." However, for much of the compiler's history nothing has prevented you from doing it and a few users made clever use of it in the meantime. By extension, if = couldn't be overloaded for a class, I don't believe == or != should be able to be either.

All that said, I wasn't sure whether we ever officially prevented it or not and, from what I'm seeing, it looks as though we might not issue an error but might not call such a user overload either? TIO

[BryantLam]

Makes sense. The compiler should definitely be issuing errors on these operators if they're not intended to be overloaded. If there's a gray area in the language, just choose the restrictive option and fix it later if it makes sense.

[bradcray]

The compiler should definitely be issuing errors on these operators if they're not intended to be overloaded.

Agreed.

[LouisJenkinsCS]

I don't like restrictions like this at all. == and != should 100% be allowed on classes as they can compare in a way that is similar to records. As well, why not allow the user to overload what they want, I strongly dislike arbitrary restrictions (I actually used operator overloading for symbolic variables and those require inheritance, which record inheritance has been disallowed for a while now).

[LouisJenkinsCS]

class someClass { var x : int; }
proc !=(c : someClass, val : integral) : bool { return c.x != val; }
proc ==(c : someClass, val : integral) : bool { return c.x == val; }
proc =(ref c : someClass, val : integral) : void { c.x = val; }

Would that be disallowed under the new restrictions? If so, 1000% against it.

[BryantLam]

Related #5358 since this discussion is morphing into a duplicate of that issue.

This problem is somewhat a manifestation of the records vs. classes distinction and the different supported features between the two, e.g., inheritance as @LouisJenkinsCS points out.

why not allow the user to overload what they want

While I won't be the one to advocate for this position: if we are to consider allowing overloading == and friends on classes, one issue I can think of is that classes need some way to compare against nil. Python has the is operator for an is None comparison. Maybe we explicitly add overloads so that these related operators all take a RHS of NilType or whatever.

There are other problems noted in #5358.

The issue here might be limited to just the operators ==, !=, and = when the LHS and RHS are both the same class type. Louis's example could conceivably work even if we forbid the former. Using Louis's code example, this code might be okay:

var x = new owned someClass(1);
var y = new owned someClass(2);
x = 10;
writeln((x, y));

// This gets tricky if you ever begin to believe that a class behaves like a record...
x = y; // original x gets dropped. y is moved.
writeln((x, y));

Also, it gets super weird once you're allowed to overload on not the same class type but rather two different class types.

class Composite {
  var x: owned Element;
}
class Element {
  var x: int;
}
proc =(ref lhs: borrowed Composite, rhs: owned Element) /* Or equivalent signature for it to work. */ {
  lhs.x = rhs;
}

var a = new owned Composite();
a = new owned Element();

Maybe that's okay?

[LouisJenkinsCS]

I think that there should be default overloads if one is not defined for the user. For example, by default you get pointer-comparison behavior by default for <, >, <=, >=, ==, and !=, and copy-initialization for =. This can be done for NilType and between classes of the same types. However the user's overloads will _always_ get the last say, no exception.

[mppf]

For example, by default you get pointer-comparison behavior by default for <, >, <=, >=, ==, and !=, and copy-initialization for =

You don't get copy-initialization for classes. It simply does not exist for classes. Instead you get aliasing (both variables point to the same instance). Note also that copy-initialization is not the same as assignment.

In my view, this is the fundamental difference between records and classes, and I think it's reasonable to ask Chapel libraries to follow that pattern with their types - at the very least to communicate to humans which pattern is being followed - i.e. if it's a by-value or by-reference type.

All that said this recent discussion is all is based upon the argument that "If overloading = isn't allowed on classes, overloading == should also not be allowed". I think it'd be reasonable to argue against that argument. For example, if in your experience @LouisJenkinsCS, you have wanted to create classes that provided all these operator overloads, but you are happy with = aliasing, then that would suggest it might be worthwhile. (In contrast, if you need = to be by value, then I think that means you should be using a record. And if you are finding using a record awkward with inheritance patterns, that means there is some other missing piece in the language. )

[LouisJenkinsCS]

Okay, I guess I'll concede to = overloading not being allowed on classes. Question though: what about things like += and -=, etc? Those are by value

[LouisJenkinsCS]

I guess an alternative would be to have a simple record wrapper that just overloads the operators and passes them directly to the underlying class instance it is wrapping. Feels like more work though.

[mppf]

Question though: what about things like += and -=, etc?

I don't think these operations necessarily introduce aliasing for classes and so I'd expect them to be allowed. For example, you might have MyInteger as a class type and then implement proc +=(lhs: MyInteger, rhs: int). That would modify the instance rather than modifying the instance pointer passed to it.

[LouisJenkinsCS]

Okay, so MyInteger(1) != MyInteger(2) and MyInteger(1) == MyInteger(1) is bad, but MyInteger(1) == 1 and MyInteger(1) != 2 is good?

[mppf]

I was talking specifically about +=. I think it's debatable whether we should allow == or !=.

Edit: Additionally, I wasn't trying to say that proc +=(lhs: MyInteger, rhs: MyInteger) couldn't be supported; just rather to describe a case where it seemed reasonably clear to me that += was OK.

[BryantLam]

A couple of loosely defined proposals to allow overloading ==, etc. that seem reasonable, but I'm not necessarily insisting on any of these:

1. With nilability, the pointer-comparison case could be implemented with C? instead of C;
   i.e., proc ==(lhs, rhs: C?). That way, Chapel could allow overloading ==(C) but not ==(C?). When doing a comparison on C?, if neither side is nil, then ==(C) will be called, or something.

2. Classes all extend from object, so Chapel could allow overloading ==(C) and the pointer-comparison case could be handled by casting C to object;
   i.e., if foo :object == nil.
   The compiler could recognize this pattern as a nil check for optimization purposes.

3. Add a Python-style is operator. Though this might be too much of a stretch.

I do sympathize with the MyInteger example in that there should be a way to overload for a by-value comparison. This is relevant for parser trees where you want to do by-value comparisons most of the time.

[bradcray]

This may or may not be helpful, but this conversation about defining == on classes reminds me of a related perennial conversation on arrays about whether == should return true/false or "array of true/false". Each time this has come up (by someone wishing for the former), we've decided to stick with the second interpretation for the == operator, but have offered up equals() as a means of getting the former behavior. This maybe only becomes compelling if/when equals() is defined on most / all types, where the default implementation of equals() could simply call == for most (value) types.

I bring this up here because I could imagine a world in which == always did the "is this the same instance" comparison for classes but equals() did the "are these values the same" comparison (and a class author would provide the implementation for their class).

[LouisJenkinsCS]

Prototype!

proc object.equals(other : object) {
    return __primitive("_wide_get_locale", this) == __primitive("_wide_get_locale", other) && 
    __primitive("_wide_get_addr", this) == __primitive("_wide_get_addr", other);
}

class C {
    var x;
}

var c1 = new C(1);
var c2 = new C(1);
var c3 = c1;
writeln(c1.equals(c1));
writeln(c1.equals(c2));
writeln(c1.equals(c3));

TIO

false
true

Also allow overloads...

proc object.equals(other : object) {
    return __primitive("_wide_get_locale", this) == __primitive("_wide_get_locale", other) && 
    __primitive("_wide_get_addr", this) == __primitive("_wide_get_addr", other);
}

class C {
    var x;

    proc equals(other : this.type) {
        return this.x == other.x;
    }
}


var c1 = new C(1);
var c2 = new C(1);
var c3 = c1;
writeln(c1.equals(c1));
writeln(c1.equals(c2));
writeln(c1.equals(c3));

TIO

true
true

Note: No override keyword needed as the type takes priority.

[bradcray]

[written before Louis's post just above showed up in response to Bryant's previous comment]:

Bryant, in your comment above, I find item 1 a little worrisome, in that it seems that == when applied to two classes would evaluate to "are these the same instance?" if either was nilable, but "are these the same value?" if neither was, which feels a bit wiggly / inconsistent / surprising.

If we were to support user-defined == and != on classes, I find item 2 a bit reassuring (that there is still a way to do "same instance" checks on them), though also a bit of a hassle (e.g., I wonder how much code we have in the compiler and modules that would need to be updated to do those cast-then-checks; and I wonder how long it would take me to train myself to use the cast when I wanted to be safe in a generic context).

Item 3 is interesting (I wasn't familiar with Python's is) and feels somewhat similar to (though the dual of?) my equals() proposal.

I have to admit I don't find the MyInteger example compelling because it seems obvious to me MyInteger should be a record / value type (or, at least, it's not obvious to me why it shouldn't be). The parse tree example is more compelling, though even there, I wonder whether the right approach is to make the class-based graph wrapped by a record where the record supports ==.

[LouisJenkinsCS]

because it seems obvious to me MyInteger should be a record / value type

This feels more like a "hammer and nail" problem. There are valid reasons to use a class over a record, such as wanting a single centralized value. Also a wrapper for a primitive, like Java's, provides a concrete nil value. Also the wrapper becomes potential mutable and can be moved around and escape the scope of the function it was allocated in (unlike, say, a ref)

[mppf]

I really like the idea of == and != always meaning pointer comparison for classes and .equals() being available for overriding / overloading. Somebody wanting to have a custom == on a type with reference semantics can still implement that by creating a record wrapping the class instance. Such a record can conceptually provide reference semantics.

I like the way that .equals() can allow overriding, too, so that the dynamic type of the class can be respected. We don't get that with ==. (Of course it does run into the perennial single-dispatch problem for binary methods).

[LouisJenkinsCS]

Personally I dislike not being able to overload == and !=, but I can live without it. Only issue is that this will fundamentally break a lot of existing code that uses == or != so I _really_ hope a compiler error is thrown from now on. However after reviewing DefaultComparator.compare, it doesn't use == or != but instead just > and <, so it might be okay for programs using comparators to compare data of types that it is generic on.

https://github.com/chapel-lang/chapel/blob/c02f95352e6ff8b8fb5d5431d3407bcd16584a82/modules/packages/Sort.chpl#L1581-L1585

[bradcray]

Based on where we left this conversation, I'm working on a PR that makes user-defined == and != overloads between classes illegal.

[bradcray]

In issue https://github.com/chapel-lang/chapel/issues/16830, I ask whether these restrictions on == and != (and ==) should only be between class/class pairs, or between class/anything and anything/class pairs as well.

[bradcray]

Uh-oh, I'm just now realizing that Arkouda makes use of == and != overloads on classes...

[bradcray]

I've filed an issue seeking input about this on the Arkouda repo (https://github.com/mhmerrill/arkouda/issues/603).

I've also been slowly working my way through other languages to see what they do, where (if I've got my facts straight) the summary is:

• C++: supporting ==/!= overloads on the equivalent of our classes would essentially mean defining them for pointer types, which is not supported
• Java: doesn't support overloading any operators
• C#: does not support overloading = but does support overloading ==/!=, though doing so could be considered dangerous/confusing (e.g., see this SO Q&A)
• Swift does support overloading ==/!= on classes, but uses distinct operators (===/!==) to determine whether two class objects point to the same thing
• Python does support overloading ==/!= and requires using is to determine whether two variables refer to the same object
• Rust... I haven't quite been able to figure out (and it seems as though the story may have changed over time?)

As usual, Swift suggests a potentially interesting path forward to me if someone (say from the Arkouda, Python, C#, or Swift community) really came down hard on wanting ==/!= between objects in Chapel: We could potentially (1) support ==/!= and ===/!== operators in Chapel, (2) have the default ==/!= operators call the ===/!== cases by default, (3) permit users to overload ==/!=, but not ===/!==. This approach would have the benefit of keeping existing (user) code working while permitting users to create their own comparison overloads. However, it would have the downside of requiring us to examine our uses of == and != in internal/standard modules to see which ones would need to be converted to ===/!== (e.g., I'm imagining hash tables would... unless we decided that hash equivalence should be based on the user's == operators rather than object identity). Beyond potentially making Arkouda/Python/C#/etc. programmers happier, this would also have the benefit of removing the asymmetry between being able to define <=, >=, etc. on classes, but not == and !=.

[bradcray]

I've forked this last idea about "Chapel could support ===/!== into its own issue: #16853

[bradcray]

Arkouda devs, coming from the Python viewpoint, have pushed back on this issue's proposal, advocating for distinct ways to compare class instances in the event that users have overloaded ==/!=. [edit: other users have expressed a preference for the status quo as well].

[BryantLam]

I'll close this for now in deference of the conversation occuring in #16853 that would resolve this issue.