The same method is dispatched for instances of a type and the Atom representing the type

[radeusgd]

The current issue

The specifics of the issue have changed a bit over time. See this comment below. But the root cause is more or less the same - in some scenarios (not as common as back in the day), the same method is dispatched for both atom instances and types when to_text is called, and this causes issues when trying to format text containing Types.

The (old) deeper issue

An example of the deeper issue is that the to_text method dispatches to the same ErrorToTextNode both for the DataflowError instances and for the Error Atom which is their 'type'. We should probably be able to dispatch differently.

As @iamrecursion pointed out:

Two issues:

• We currently resolve methods on AtomConstructor where we shouldn't.
• We wrongly treat typesets such as Number as AtomConstructors where we shouldn't.

The fix:

• Redefine dispatch on AtomConstructor to only dispatch methods on it defined on Any.
• Introduce a third notion of a type, Typeset that is used where we currently "wrongly" use atom constructors.
• Register the typesets in scope.
• Implement method definitions on typesets.
• Disallow method dispatch on typesets.

This may need to be updated based on updates to the typesystem design too.

The original bug

A ClassCastException from within GetFieldNode is exposed to the user.

Steps to Reproduce

1. Execute the following code:

from Base import all

type Bar
   type Bar foo

   to_text : Text
   to_text = this.foo.to_text

main = 
    foo = Bar.to_text
    IO.println foo

Expected Result

Not really sure, since we have this weird semantics of static methods (i.e. that extension methods defined on a type are applicable not only on its atoms but also on its constructor).

Certainly the code should not expose such an internal error.

It seems reasonable for the constructor to have its own to_text that would print the name of the constructor, although that is likely not going to interact well with the 'static methods semantics' that we have.

I'm not aware of any documentation regarding semantics of this kind of static methods, so I think it should be documented (and possibly redesigned if necessary). If the current design of static methods (which is very weird) is to stay, I think referencing this in a call that is resolved on an atom constructor should probably throw some intuitive error.

Actual Result

Execution finished with an error: java.lang.ClassCastException: class org.enso.interpreter.runtime.callable.atom.AtomConstructor cannot be cast to class org.enso.interpreter.runtime.callable.atom.Atom (org.enso.interpreter.runtime.callable.atom.AtomConstructor and org.enso.interpreter.runtime.callable.atom.Atom are in unnamed module of loader com.oracle.graalvm.locator.GraalVMLocator$GuestLangToolsLoader @7161d8d1)
        at <java> org.enso.interpreter.node.expression.atom.GetFieldNode.execute(GetFieldNode.java:34)
        at <java> org.enso.interpreter.node.expression.atom.GetFieldNode.execute(GetFieldNode.java:11)
        at <enso> argument<0>(src/Main.enso:7:14-21)
        at <enso> Bar.to_text(src/Main.enso:7:14-29)
        at <enso> Main.main(src/Main.enso:10:11-21)

Enso Version

Enso Launcher
Version:    0.2.4-SNAPSHOT
Built with: scala-2.13.3 for GraalVM 20.2.0
Built from: wip/rw/database-library* @ 5b680ce57b56d67b4e392a416459b8da187fd03e
Built on:   Linux (amd64)
Current default Enso engine: 
Enso Compiler and Runtime
Version:    0.2.7-SNAPSHOT
Built with: scala-2.13.5 for GraalVM 21.0.0.2
Built from: wip/rw/database-connection* @ d74020cf946184977ba850ec565a252fd0210289
Running on: OpenJDK 64-Bit Server VM, GraalVM Community, JDK 11.0.10+8-jvmci-21.0-b06
            Linux 4.15.0-136-generic (amd64)

[radeusgd]

I did not put the breaking/non-breaking label because I think that highly depends on the chosen solution.

[iamrecursion]

Two issues:

• We currently resolve methods on AtomConstructor where we shouldn't.
• We wrongly treat typesets such as Number as AtomConstructors where we shouldn't.

The fix:

• Redefine dispatch on AtomConstructor to only dispatch methods on it defined on Any.
• Introduce a third notion of a type, Typeset that is used where we currently "wrongly" use atom constructors.
• Register the typesets in scope.
• Implement method definitions on typesets.
• Disallow method dispatch on typesets.

[iamrecursion]

I've just run into this again. It's annoying.

[radeusgd]

Updated the issue to point to the broader underlying issue.

As also discussed at https://discord.com/channels/401396655599124480/955900131197194332 it would be good if we could see how the new typesystem design influences this issue and if it takes it into account at all (as it may be worth considering, because the current behaviour is unintuitive and leading to issues as the one reported here).

cc: @wdanilo @ekmett @kustosz

[radeusgd]

This issue still persists (although it changed a bit), I just ran into it today when the test suite crashed trying to display a matcher.to_text where matcher was a Type whose to_text was overridden (SQL_Error in this case).

To reproduce it, run:

from Standard.Base import all

type Foo
    Value x y

type Bar
    Value x y

    to_text self = 
        "{{Bar x="+self.x.to_text+"; y="+self.y.to_text+"}}"
    
main =
    f_inst = Foo.Value 1 2
    f_typ = Foo

    IO.println f_inst.to_text
    IO.println f_typ.to_text
    IO.println "Atom = "+f_inst.to_text
    IO.println "Type = "+f_typ.to_text

    b_inst = Bar.Value 3 4
    b_typ = Bar

    IO.println b_inst.to_text
    IO.println b_typ.to_text
    IO.println "Atom = "+b_inst.to_text
    # This will crash, because `b_typ.to_text` returns a value that is NOT a Text, so `+` operator fails with a Type Error.
    IO.println "Type = "+b_typ.to_text

it currently prints:

(Foo.Value 1 2)
Foo
Atom = (Foo.Value 1 2)
Type = Foo
{{Bar x=3; y=4}}
Bar.type.to_text[static-totext.enso:9-37]
Atom = {{Bar x=3; y=4}}
Execution finished with an error: Type error: expected `str` to be Text, but got Function.
        at <enso> Text.+(Internal)
        at <enso> static-totext.main<arg-1>(static-totext.enso:28:16-38)
        at <enso> static-totext.main(static-totext.enso:28:5-38)

As we can see, we cannot rely on x.to_text being a Text, because in case of this type Bar it is an unapplied function. This is problematic in places like the test suite that is trying to print a name of the type.

As we can see in this example, this already works correctly for atoms that rely on the default to_text implementation. But once it is overridden (which is a pretty normal thing to do for an atom), the static dispatch mode breaks.

I think the correct solution here would be to ignore the override in static mode, a bit like we are doing for atoms without an override. If we are dispatching on a Type and not an Atom Instance, we should probably just keep the default behaviour.

This makes handling of the to_text method a bit special, but I think it is what we need to be able to properly display Types in places like the test suite or error handling code.

[JaroslavTulach]

The recent @radeusgd endeavor seems like the same problem as reported by me two weeks ago:

• IO.println: Type error: expected str to be Text, but got Function #5961

yes, the inability to use println to display everything is annoying.