Bad error message when importing .did that isn't a service

[nomeata]

Users out there ran into an exception: https://forum.dfinity.org/t/build-error-with-imported-actor-class/1931

They built a canister using actor class…, which produced this .did:

type anon_class_1_1 = 
 service {
 };
service : () -> anon_class_1_1

which then tripped this code:

    match actor with
    | Some {it=ServT ms; _} ->
       let fs = List.map (check_meth env) ms in
       let fs = M.Env.fold (fun id t fs ->
         match t with
         | M.Con (c, _) -> M.{lab = id; typ = M.Typ c}::fs
         | _ -> assert false) !m_env fs in
       M.Obj (M.Actor, List.sort M.compare_field fs)
    | None -> assert false
    | _ -> assert false 

These assert false should produce more helpful error messages.

(Of course, we also should support importing canister classes, but that's a bigger thing; this issue is just about proper error messages.)

[nomeata]

@stanleygjones not sure if this is counts as a bugfix (not story-worthy), or if it would constitute a developer-facing improvement worth of a few points.

[crusso]

It's interesting to note that they only reason they were trying to do this is to get access to the installer of an actor, which you can't do except by authoring it as an actor class. Perhaps we should have put the context pattern on the actor keyword after all or expose the primitive for getting access to the installer/caller.

[nomeata]

Actually, one could argue that this is more an issue for dfx, at least if one follows the philosophy that dfx (by deploying) turns a canister class into a canister instance, and should therefore strip the parameters from the Candid file.

But we should still provide a better error message.

[chenyan-dfinity]

hmm, should this be an error or we can be more permissive that when import canister: for an actor class, it refers to the canister instantiated by the actor class (strip out the init arguments); when import class: for an actor class, it refers to the uninstantiated actor class.

[nomeata]

That's the pragmatic, non-principled approach ;-)

I think we had that in the thread where we discussed this: conceptually, an IDL for a service instance has no parameters. Whether we strip them in dfx or (implicitly) when parsing is a minor matter of the sort that can cause long and heated debate.

[rossberg]

Yes, please let's not confuse the domains of X and Y->X, totally different beasts. Even if it seems "pragmatic", I fear it doesn't help users in understanding the difference between the two. And it is likely to get in the way of making actor classes first-class later.

[nomeata]

People keep running into this (https://forum.dfinity.org/t/motoko-compiler-bug/5095). We should at least produce a helpful error message.

[crusso]

That's weird, I too thought this had been fixed (or, at least, the error improved).

[nomeata]

Maybe there was a hard fork in the universe's reality?

[chenyan-dfinity]

Weird, I also remember there was a PR to improve the error message....

[crusso]

hmm, should this be an error or we can be more permissive that when import canister: for an actor class, it refers to the canister instantiated by the actor class (strip out the init arguments); when import class: for an actor class, it refers to the uninstantiated actor class.

I'm with @chenyan-dfinity on this one. If the code is written as a class then importing a canister of that class could just strip the argument. We don't support importing of actor classes via import "canister:" and using import "class:" would arguably be more appropriate anyway.

[crusso]

The confusion is already baked into to the language design. A class declaration both names the type of its instances (an actor) and the name of a constructor (a function). Canister import can just be interpreted as importing an actor or instance of a class.

[matthewhammer]

this issue is just about proper error messages.

Reminder.