Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

CorrespondenceMutator + assignment to item operator #4148

Closed
CeylonMigrationBot opened this issue Jul 24, 2014 · 101 comments
Closed

CorrespondenceMutator + assignment to item operator #4148

CeylonMigrationBot opened this issue Jul 24, 2014 · 101 comments

Comments

@CeylonMigrationBot
Copy link

[@lucaswerkmeister] Summary: Polymorphic operator support for array[index] = val; map[key] = item;.

I suggest that we add the following interfaces

shared interface CorrespondenceMutator<in Key,in Item>
        given Key satisfies Object {
    shared formal void set(Key key, Item item);
}

shared interface MutableCorrespondence<in Key,Item>
        satisfies Correspondence<Key,Item> & CorrespondenceMutator<Key,Item>
        given Key satisfies Object {}

and define that for

  • any expression cor of type CorrespondenceMutator<Key, Item>,
    • (given that cor is on the operator precedence level of “member invocation and selection, index, subrange, postfix increment, postfix decrement” or a Primary, i. e., that it’s syntactically sound to write cor[key])
  • any expression key of type Key, and
  • any expression item of type Item,

an assignment operator expression with the left-hand side cor[key] and the right-hand side item is syntactical sugar for cor.set(key, newValue), where newValue is

  • item for an “assign” expression (=)
  • oldValue OP item for any of the other assignment expressions (+=, *=, &=, etc.), where
    • oldValue is cor[key],
    • cor must therefore also be of type Correspondence<Key, Item> (typically MutableCorrespondence<Key, Item>),
    • OP is the operator of the assignment (+, *, &, etc.), and
    • that operator therefore places additional restrictions on the type of Item (Summable, Numeric, Set, etc.),

and that this also “carries over” to increment or decrement operators (which are already defined in terms of an “assign” expression).

This would, for example, allow code that operates on arrays to look very similar to its Java counterpart:

array[index] = val;
array[index += step] += increment;

but it would also be useful for other kinds of correspondences:

list[index] = val;
map[key] = item;

References:

[Migrated from ceylon/ceylon-spec#1042]

@CeylonMigrationBot
Copy link
Author

[@lucaswerkmeister] Come to think about it, this is rubbish for assignment expressions other than “assign” expressions, because oldValue doesn’t necessarily exist, and we don’t (yet) have a mechanism to choose a default value based on the type (#4005). In other words, we can’t say what foo[bar]++ does if foo[bar] is null.

@CeylonMigrationBot
Copy link
Author

[@gdejohn] How about CorrespondenceMutator.set() returns the old item associated with the given key (like MapMutator.put()), where null indicates no item was present. Then you can do all of the compound assignments, and if foo[bar] doesn't exist, nothing happens (and of course can you check for that).

foo[bar]++ else foo[bar] = 0;

@CeylonMigrationBot
Copy link
Author

[@lucaswerkmeister] I don't follow. We already need to know the value when calling put, how will its return value help us?

-----Original Message-----
From: "Griffin DeJohn" [email protected]
Sent: ‎27.‎12.‎2014 04:42
To: "ceylon/ceylon-spec" [email protected]
Cc: "Lucas Werkmeister" [email protected]
Subject: Re: [ceylon-spec] CorrespondenceMutator + assignment to item operator(#4148)

How about CorrespondenceMutator.set() returns the old item associated with the given key (like MapMutator.put()), where null indicates no item was present. Then you can do all of the compound assignments, and if foo[bar] doesn't exist, nothing happens (and of course can you check for that).
foo[bar]++ else foo[bar] = 0;

Reply to this email directly or view it on GitHub.=

@CeylonMigrationBot
Copy link
Author

[@gdejohn] I'm saying allow foo[bar]++ and friends, even though we don't know that foo[bar] exists. If foo[bar] doesn't exist, then foo[bar]++ has no effect. The old item should be returned so that the programmer knows if it was successful or not (with null indicating failure). I don't think it's a perfect solution, but I'm just throwing it out there because I don't see any other way.

@CeylonMigrationBot
Copy link
Author

[@luolong] I am not sure foo[bar]++ would even work if foo[bar] returns null.

The thing is that since type of foo[bar] is presumably Bar? (or more explicitly Bar|Null), so you can not apply postfix ++ operator on it without hitting a type error.

@CeylonMigrationBot
Copy link
Author

[@gdejohn]

The thing is that since type of foo[bar] is presumably Bar? (or more explicitly Bar|Null), so you can not apply postfix ++ operator on it without hitting a type error.

I understand that, but this still wouldn't be regular even if the type of foo[bar] was exactly Bar, right? I'm saying, if you want compound assignment in this case, you gotta do something special. (I'm not sure if I want it that badly, I'm just spitballing.) Namely, define foo[bar]++ and friends to have no effect and evaluate to null when foo[bar] doesn't exist. I'm not sure how else to explain it. @gavinking, is this making any sense?

@CeylonMigrationBot
Copy link
Author

[@gavinking] Honestly I don't like the sound of it. Seems to be hiding just too much implicitness inside an innocuous-looking ++ operator.

@CeylonMigrationBot
Copy link
Author

[@gdejohn] Yeah, that's a reasonable complaint.

@CeylonMigrationBot
Copy link
Author

[@Zambonifofex]
NullPointerException for the win?
Kidding, of course... :-P

@quintesse
Copy link
Contributor

I created an implementation for this in the typechecker and the JVM backend.
It only supports simple indexing, not ranged because the latter would be an enormous amount of work on the JVM backend for something that I guess is of only marginal value.

@gavinking could you take a look at the language/typechecker part to see if things seemokay?

@chochos would you be willing to create the JS backend side for this?
(There's also a weird problem with the JS compiler when I try to add the mutator interface to Array, but we can look at that afterwards)

@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

Well, that's not exactly the same, they don't test boxing or the various types of LHS.

@quintesse
Copy link
Contributor

Btw, after I closed this @jvasileff opened #6435 which made me realize the current implementation is crap anyway. Honestly I'm getting lost in the complexity of our expression builders and have no idea what the proper way to implement this would be. That's the cause of #6435, because I somehow circumvent the correct way of generating all the proper accesses of local/class/toplevel variables.

@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

OK, fixed for the JVM.

@FroMage FroMage reopened this Aug 25, 2016
@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

I added the interfaces as agreed, but they need documentation, can you help @gavinking ? Then @chochos needs to do the JS magic.

I also fixed the jvm backend so that the code works wrt any expression and boxing/primitive types, and added backend tests also for interop.

I'll open an SDK issue to add this to the SDK.

@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

Done in SDK: eclipse-archived/ceylon-sdk#607

@FroMage FroMage assigned chochos and unassigned FroMage Aug 25, 2016
@gavinking
Copy link
Contributor

gavinking commented Aug 25, 2016

So have we tested this for all of the following:

  • Array
  • Java arrays
  • ListMutators
  • MapMutators
  • Java Lists
  • Java Maps

@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

Yes.

FroMage added a commit that referenced this issue Aug 25, 2016
@FroMage
Copy link
Contributor

FroMage commented Aug 25, 2016

Thanks @chochos !

@FroMage
Copy link
Contributor

FroMage commented Aug 26, 2016

Actually this is not BC: we can't run 1.2.2 ceylon.collection in 1.2.3 because Array.set is not defined as returning Object (the case when the method is defined in a Ceylon interface) when it used to return void before (the case when the method was defined in a Ceylon class), so the runtime blows up with a NoSuchMethodError.

@FroMage
Copy link
Contributor

FroMage commented Aug 26, 2016

Fixed with a terrible hack to generate a void bridge method :(

@FroMage FroMage closed this as completed Aug 26, 2016
@quintesse
Copy link
Contributor

👍

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants