Chapel: Overloading assignment (=) on class types

[bradcray]

The Chapel compiler currently permits users to overload assignment on class types. This seems questionable to me in a few ways: First, since assignment of class variables is like an assignment between pointers-to-objects, it seems suspicious to let users cause it to mean something else -- i.e., it seems as though it could interfere with compiler-introduced temps/copies or _possibly_ with type inference (though I can also imagine how that might work out). In addition, it seems as though it could be confusing to readers of the code who know they're dealing with classes and think they know what assignment should mean, only to find it means something else. By way of analogy, C++ permits assignment overloads between structs but not between pointers-to-structs (unless I'm mistaken).

If we wish to make assignment overloads on classes illegal, the compiler should flag it as such and stop letting it pass through. If we want to support it, we should do more testing to make sure it works as expected.

test/classes/hilde/freeAssignment.chpl is a future that poses this semantic question and demonstrates that even when assignment is overloaded, it doesn't always behave as a user might expect.

[bradcray]

If it's not clear from the above, I'm in favor of making assignment overloads on classes illegal, at least for the time being and until we have a compelling case that argues in favor of supporting it.

[mppf]

The TOML module has a bunch of = overloads where the lhs is a class type, e.g. https://github.com/chapel-lang/chapel/blob/32a5599d3525ecf7bfa99c36997b4c825ba62aca/modules/packages/TOML.chpl#L468-L476

Even if we decided to allow these since they are not class-to-class assignment but use different types, I don't think it's a very readable way to write what is happening here. I think a developer unfamiliar with the Toml module will see = and be confused. (And I particularly don't like the idea of = allocating a nil LHS).

This is used in generally two ways and I have in mind a different strategy for each.

setting up a new Toml variable

var toml: unmanaged Toml
toml = myString;

The above runs into problems with Toml being not-nilable. We will simply replace this with

var toml = new unmanaged Toml(myString);

updating a Toml subtree

depTree[package]["name"] = myString;

We will update this to look like

depTree[package].set("name", myString);

@ben-albrecht @Spartee - does this sound OK?

[bradcray]

I agree it'd be nice to not have TOML rely on this feature.

Does anyone know: What would be the level of effort of having the compiler issue errors for assignment overloads between class types since we generally seem skittish about the idea when it comes up, and it'd help avoid surprises if/when we outlawed it in the future?

[bradcray]

I needed to do something constructive after weeks of release and reporting, and was thinking about what breaking changes remain, so took a quick stab at putting a branch for this together which seems to work for some simple cases. Running testing to see whether we have other cases that rely upon it (I have a vague memory that studies/amr may).