dfinity / motoko

Simple high-level language for writing Internet Computer canisters
Apache License 2.0
515 stars 97 forks source link

Bug: Stateful desugaring of anonymous classes leaks into types #1898

Closed crusso closed 3 years ago

crusso commented 4 years ago

Generating fresh identifiers for desugaring is all very well, but it makes me uneasy that the identifiers are leaking into type, and inconsistently at that:

Consider:

[nix-shell:~/motoko/test]$ rlwrap moc
Motoko compiler (revision 0.4.2-100-g973483c2-dirty)
> module X = { public class(){} };
type anon-class-1.21 = {}
let X : module {...}
> X;
{anon-class-1.21 = func} : module {type anon-class-1.21 = {}; anon-class-1.21 : () -> anon-class-1.21}
> module X = { public class(){} };
type anon-class-3.21 = {}
let X : module {...}
> X;
{anon-class-3.21 = func} : module {type anon-class-3.21 = {}; anon-class-3.21 : () -> anon-class-3.21}
> module Y = { public func(){} };
let Y : module {...}
> Y
  ;
{} : module {}
> 

Note the two identical module declarations X producing values of different types (due to gensym) and the inconsistency with the treatment of an anonymous function declaration.

@rossberg, @nomeata I'm not comfortable with this - are you? Should we reject anonymous identifiers escaping as field names? Is that enough?

crusso commented 4 years ago

@kritzcreek I know you love sugar...

crusso commented 4 years ago

It appears that that anonymous object and functions are deal with correctly in the parser, but classes are not and should probably have the class pushed into a block expression when anonymous (if extending that pattern). This may complicate compilation unit detection a little...

Alternatively we simply syntactically forbid anonymous classes (and anonymous imports, which seem rather pointless given that they are pure).

nomeata commented 4 years ago

My gut feeling was never quite happy around these generated ids. Do we really need them? Are they maybe just a left-over artifact from when type definitions needed to be manifestly in scope?

crusso commented 4 years ago

No, I don't recall them having anything to do with type definitions - they are just an artifice of desugaring. But I'm now convinced this is just buggy desugaring for classes - the meaning of a term (including its static approximation as a type) should never depend on the choice of bound names. I'll fix this once I land the class import PR.