Feedback on Migration syntax

[ByronBecker]

@crusso @ggreif
I just saw the new migration support, which is very exciting! https://github.com/dfinity/motoko/releases/tag/0.14.0

My high level reaction from just looking at the release notes is that using (), for example (with migration = ...) feels a bit messy. Additionally, I'm not sure how this gets explicitly attached to the persistent actor, other than being directly above it

Maybe introducing a migration module type (record), that can be passed/attached to the actor in some way would be helpful?

Right now there isn't anything in the syntax directly connecting the migration keyword to the actor, and I'm curious if this structure is built so that it can evolve over time (i.e. adding in more than just migrations, via with migration; checks; hooks; ...).

[ggreif]

Hi @ByronBecker, thanks for the feedback! Indeed we don't anticipate to people to write migration functions as lambdas behind (with migration =, it is just a possibility.

Normally the migration function will be neatly tucked away in a different module and will be applied like this:
https://github.com/dfinity/motoko/blob/master/doc/md/examples/Card-v1c.mo#L3
This one uses field punning to capture the identifier migration from the environment into the parenthetical note. The note than modifies the actor's upgrade behaviour.

The key point is that parenthetical notes apply to expressions, conveying extra information (modalities). A PR is in development to use parentheticals adding cycles to external/self message sends and to apply timeouts for best effort sends.

We will see what other modalities make sense. Regarding the syntax, I think this is quite better than Rust's [#attr ...] annotations, and in Motoko potentially each subexpression can have one. Something like (with saturate = 999999) someNat + 50_000 is conceivable, applying a cutoff on the result, etc.

So please don't stare only on that example in the release note, the common case will be much nicer.

[ByronBecker]

@ggreif The current placing feels a bit like a decorator. Also, it's strange that there's nothing that explicitly attaches that migration to the actor other than its proximity. It also makes it seems like you could do this:

import {m1} "m1";
import {m2} "m2";
import {m3} "m3";

(with migration = m1);
(with migration = m2);
(with migration = m3);
persistent actor {
  ...
};

I guess I find it slightly awkward for the migration syntax to be located outside of the actor.

I might just be on an island though here 😅, again - really excited to see this feature come out!

[ggreif]

Nope, there is at most one parenthetical prefixing an expression (here: actor). No semi goes in-between. What you can do:

import {m1} "m1";
import {m2} "m2";
import {m3} "m3";

type OldStables = { <list all old stable bindings with types that you are interested in> };
type NewStables = { <list all current stable bindings with types that want to get filled> };

(with migration = func(old : OldStables) : NewStables = { m1 old and m2 old and m3 old })
persistent actor {
  ...
  // fields from `NewStables` will be filled by migration, the others defaulted by initialiser
};

[ggreif]

I fleshed this out as a compilable example.

[ggreif]

Note that instead of spelling out OldStables, you can construct it too: func(old : M1.Old and M2.Old). Similarly for the return record type. Thus you can pretty much separate the concerns.

[crusso]

Note, the example above only type-checks in the more liberal interpreter. To actually compile this code the type definitions currently need to go into a separate file (though we could relax that).

[crusso]

Actually, I sympathise that re-using () is maybe a bit too subtle and perhaps we should use a more prominent delimiter for so called parentheticals, either (| ... |) or some other variant that sticks out more and is easy to grep for.

The reason for placing the annotation at the front of the class is to reflect the lexical scoping - the migration expression doesn't have access to constructor arguments or any fields or methods of the actor body. It also make it easer to delete the annotation and any import without disturbing the rest of the actor/class code.

For migration we first had a straw man syntax that places the migration expression after the actor keyword

persistent actor [ <exp> ] 
{
  ...
  // fields from `NewStables` will be filled by migration, the others defaulted by initialiser
};

but that reads oddly with classes and raise the question if the {caller} is bound in exp or not/

shared {caller} persistent actor [ <exp> ] class C ()
{
  ...
  // fields from `NewStables` will be filled by migration, the others defaulted by initialiser
};

On the whole, I think the prefix syntax works a little better.

Another option would have been a new system method in the class but that too has problems with breaking lexical scoping.

The rest of syntax was driven by the desire to harmonise with the cycles/timeout annotations in this PR #4608

I'm not opposed to revising the syntax of parentheticals if there's alot of pushback, but I think having the parenthetical outside of the actor/class makes sense for lexical scoping reasons.