rascal icon indicating copy to clipboard operation
rascal copied to clipboard

Keyword parameters not working on parameterized abstract data types

Open rodinaarssen opened this issue 5 years ago • 11 comments

data E[&T] = e(&T t, int j = 0);
rascal>e(1, j=2)
E[int]: e(1,j=2)
rascal>e(1, j=2).j
|prompt:///|(7,1,<1,7>,<1,8>): Undeclared field: j for E[int]
Advice: |http://tutor.rascal-mpl.org/Errors/Static/UndeclaredField/UndeclaredField.html|

My guess is that somewhere around here the lookup in the HashMap fails since E[&T] is not the same as E[int].

rodinaarssen avatar Sep 30 '19 13:09 rodinaarssen

Having fixed this, another bug is slightly more insidious:

data F[&T] = F(&T t, &T x = t);

rascal>F(1)
F[int]: F(1)
rascal>F(1).x
&T: 1
rascal>F(1,x=3).x
&T: 3

Apparantly, the type parameter is not properly instantiated by the field access implementation.

jurgenvinju avatar Sep 30 '19 13:09 jurgenvinju

This now works for normal parameters, but crashes the REPL for keyword parameters:

rascal>data F[&T] = F(&T t, &T x = t);
ok
rascal>F(1).t
int: 1
rascal>F(1, x=3).x
Unexpected (uncaught) exception, closing the REPL: 
java.lang.ClassCastException: class io.usethesource.vallang.impl.primitive.IntegerValue cannot be cast to class io.usethesource.vallang.IConstructor (io.usethesource.vallang.impl.primitive.IntegerValue and io.usethesource.vallang.IConstructor are in unnamed module of loader 'app')java.lang.ClassCastException: class io.usethesource.vallang.impl.primitive.IntegerValue cannot be cast to class io.usethesource.vallang.IConstructor (io.usethesource.vallang.impl.primitive.IntegerValue and io.usethesource.vallang.IConstructor are in unnamed module of loader 'app')
        at org.rascalmpl.repl.BaseRascalREPL.serveContent(BaseRascalREPL.java:194)
        at org.rascalmpl.repl.BaseRascalREPL.printResult(BaseRascalREPL.java:159)
        at org.rascalmpl.repl.BaseRascalREPL.handleInput(BaseRascalREPL.java:106)
        at org.rascalmpl.vscode.lsp.terminal.LSPTerminalREPL$1.handleInput(LSPTerminalREPL.java:183)
        at org.rascalmpl.repl.BaseREPL.handleInput(BaseREPL.java:180)
        at org.rascalmpl.repl.BaseREPL.run(BaseREPL.java:347)
        at org.rascalmpl.vscode.lsp.terminal.LSPTerminalREPL.main(LSPTerminalREPL.java:252)
java.io.IOException: Stream closed
        at java.base/sun.nio.cs.StreamEncoder.ensureOpen(StreamEncoder.java:45)
        at java.base/sun.nio.cs.StreamEncoder.flush(StreamEncoder.java:152)
        at java.base/java.io.OutputStreamWriter.flush(OutputStreamWriter.java:251)
        at jline.console.ConsoleReader.flush(ConsoleReader.java:1047)
        at org.rascalmpl.repl.BaseREPL.run(BaseREPL.java:382)
        at org.rascalmpl.vscode.lsp.terminal.LSPTerminalREPL.main(LSPTerminalREPL.java:252)
Rascal terminal terminated exceptionally; press any key to exit process.

rodinaarssen avatar Jan 24 '22 11:01 rodinaarssen

Ooo.. nasty. The problem is caused by the field type not being instantiated at the right time. It remains &T throughout the construction of the Result object (factory method makeResult) which used the static (but instantiated) type of a value to dispatch to the right kind of value.

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

To fix this, the code could become really expensive to run. You see the &T could be shared in complex ways between both normal fields and keyword fields. To find out the static least-upper-bound of all uses of &T we'd have to interpret all the default expressions. If we only evaluate the one we need, and not the other unused ones, we might get a more precise static type than the type-checker will offer.... I have to think about this a bit.

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

This is also curious:

rascal>data A[&T] = A(&T a = 1);
ok
rascal>A().a
int: 1

and this:

rascal>A[str] cc = A();
A[str]: A()
rascal>cc.a
int: 1

rodinaarssen avatar Jan 24 '22 11:01 rodinaarssen

The type parameter is missing in the following too:

rascal>A()
A: A()

Where I would expect A[int]: A() or maybe A[&T]: A()

rodinaarssen avatar Jan 24 '22 11:01 rodinaarssen

Right! thanks. It's all instances of the same bug; but there are different places in the implementation of ConstructorResult.fieldAccess that each of these examples trigger.

A simple fix would fix one issue, but not the other. I.e.

rascal>F(1, x=3).x
int: 3
rascal>F(1, x="2").x
java.lang.ClassCastException: class io.usethesource.vallang.impl.primitive.StringValue$SimpleUnicodeString cannot be cast to class io.usethesource.vallang.IInteger (io.usethesource.vallang.impl.primitive.StringValue$SimpleUnicodeString and io.usethesource.vallang.IInteger are in unnamed module of loader 'app')
(internal error)
	at $shell$(|main://$shell$|)

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

It becomes a question of language semantics. Are the parametrized keywords open or closed? I would think open, otherwise constructos with only keyword fields could never be open... So there are several locations to fix in the interpreter: the places where type parameters are bound which currently ignore keyword fields and the places where type parameters are used/projected which currently do not instantiate the bindings.

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

At the places where they are bound we can make a fast path for closed data-types, and a slow path for the open data-types. I suspect it is already so. The slow path will become even slower. I suspect we will have to fix vallang too.

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

Since keyword parameters are not stored with the ConstructorType but rather in the TypeStore, and match (which does all the open parameter binding for vallang) does not take a TypeStore as a parameter, I think we are in for a major API change with a big impact in the interpreter :-(

jurgenvinju avatar Jan 24 '22 11:01 jurgenvinju

Another complicating factor: concrete run-time types of default fields are dependent on actual run-time values of fields and other default field computations. However, vallang does not have programmatic access to the default field computations. Point to ponder.

jurgenvinju avatar Jan 24 '22 13:01 jurgenvinju