Ensure type ascriptions are correctly transformed to facilitate checking of more complex return types (#10229)

- Fixes #9980
- Adds some tests to ensure types like `|` or `&` (in addition to `!` from the ticket) correctly work in return type check.
- Fixes a weird behaviour where we used to avoid processing type related IR transformations inside of type ascriptions.
- Adds parentheses to type representations if they are more complex: `A | B & C` is unclear as it can either mean `A | (B & C)` or `(A | B) & C` which have different meanings. If we now have an operation with such nesting, the sub expressions are wrapped in parentheses to disambiguate.
This commit is contained in:
Radosław Waśko 2024-06-11 19:11:03 +02:00 committed by GitHub
parent a2c4d94735
commit c4b0ca8f69
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 224 additions and 27 deletions

View File

@ -89,7 +89,8 @@ type Encoding
Convert an Encoding to it's corresponding Java Charset, if applicable. Convert an Encoding to it's corresponding Java Charset, if applicable.
This method should be used in places not aware of special logic for the This method should be used in places not aware of special logic for the
Default encoding. In such places, usage of Default encoding will be forbidden. Default encoding. In such places, usage of Default encoding will be forbidden.
to_java_charset self -> Charset ! Illegal_Argument = to_java_charset : Charset ! Illegal_Argument
to_java_charset self =
self.to_java_charset_or_null.if_nothing <| self.to_java_charset_or_null.if_nothing <|
warning = Illegal_Argument.Error "The Default encoding has been used in an unexpected place (e.g. Write operation). It will be replaced with UTF-8. Please specify the desired encoding explicitly)" warning = Illegal_Argument.Error "The Default encoding has been used in an unexpected place (e.g. Write operation). It will be replaced with UTF-8. Please specify the desired encoding explicitly)"
Warning.attach warning (Encoding.utf_8.to_java_charset) Warning.attach warning (Encoding.utf_8.to_java_charset)
@ -97,7 +98,8 @@ type Encoding
## PRIVATE ## PRIVATE
Convert an Encoding to it's corresponding Java Charset or null if it is the Default encoding. Convert an Encoding to it's corresponding Java Charset or null if it is the Default encoding.
This method should only be used in places where a null Charset is expected - i.e. places aware of the Default encoding. This method should only be used in places where a null Charset is expected - i.e. places aware of the Default encoding.
to_java_charset_or_null self -> Charset ! Illegal_Argument = case self of to_java_charset_or_null : Charset | Nothing ! Illegal_Argument
to_java_charset_or_null self = case self of
Encoding.Value charset_name -> Encoding.Value charset_name ->
Panic.catch UnsupportedCharsetException (Charset.forName charset_name) _-> Panic.catch UnsupportedCharsetException (Charset.forName charset_name) _->
Error.throw (Illegal_Argument.Error ("Unknown Character Set: " + charset_name)) Error.throw (Illegal_Argument.Error ("Unknown Character Set: " + charset_name))

View File

@ -2,7 +2,7 @@ package org.enso.compiler.pass.desugar
import org.enso.compiler.context.{InlineContext, ModuleContext} import org.enso.compiler.context.{InlineContext, ModuleContext}
import org.enso.compiler.core.ir.expression.{Application, Operator} import org.enso.compiler.core.ir.expression.{Application, Operator}
import org.enso.compiler.core.ir.{Expression, Module, Type} import org.enso.compiler.core.ir.{Expression, Module}
import org.enso.compiler.pass.IRPass import org.enso.compiler.pass.IRPass
import org.enso.compiler.pass.analyse.{ import org.enso.compiler.pass.analyse.{
AliasAnalysis, AliasAnalysis,
@ -71,8 +71,6 @@ case object OperatorToFunction extends IRPass {
inlineContext: InlineContext inlineContext: InlineContext
): Expression = ): Expression =
ir.transformExpressions { ir.transformExpressions {
case asc: Type.Ascription =>
asc.copy(typed = runExpression(asc.typed, inlineContext))
case Operator.Binary(l, op, r, loc, passData, diag) => case Operator.Binary(l, op, r, loc, passData, diag) =>
Application.Prefix( Application.Prefix(
op, op,

View File

@ -106,16 +106,14 @@ case object TypeFunctions extends IRPass {
* @return `expr`, with any typing functions resolved * @return `expr`, with any typing functions resolved
*/ */
def resolveExpression(expr: Expression): Expression = { def resolveExpression(expr: Expression): Expression = {
expr.transformExpressions { expr.transformExpressions { case app: Application =>
case asc: Type.Ascription => asc val result = resolveApplication(app)
case app: Application => app
val result = resolveApplication(app) .getMetadata(DocumentationComments)
app .map(doc =>
.getMetadata(DocumentationComments) result.updateMetadata(new MetadataPair(DocumentationComments, doc))
.map(doc => )
result.updateMetadata(new MetadataPair(DocumentationComments, doc)) .getOrElse(result)
)
.getOrElse(result)
} }
} }

View File

@ -897,6 +897,33 @@ public class SignatureTest {
assertFalse("false & false", compute.execute(false, false).asBoolean()); assertFalse("false & false", compute.execute(false, false).asBoolean());
} }
@Test
public void unionInArgument() throws Exception {
var uri = new URI("memory://binary.enso");
var src =
Source.newBuilder(
"enso",
"""
from Standard.Base import all
foo (arg : Integer | Text) = arg
""",
uri.getAuthority())
.uri(uri)
.buildLiteral();
var module = ctx.eval(src);
var ok1 = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo 42");
assertEquals(42, ok1.asInt());
var ok2 = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo 'Hi'");
assertEquals("Hi", ok2.asString());
try {
var v = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo []");
fail("Expecting an error, not " + v);
} catch (PolyglotException ex) {
assertTypeError("`arg`", "Integer | Text", "Vector", ex.getMessage());
}
}
@Test @Test
public void unresolvedReturnTypeSignature() throws Exception { public void unresolvedReturnTypeSignature() throws Exception {
final URI uri = new URI("memory://neg.enso"); final URI uri = new URI("memory://neg.enso");
@ -1209,6 +1236,127 @@ public class SignatureTest {
} }
} }
@Test
public void returnTypeCheckErrorSignature() throws Exception {
final URI uri = new URI("memory://rts.enso");
final Source src =
Source.newBuilder(
"enso",
"""
from Standard.Base import Integer
import Standard.Base.Errors.Illegal_State.Illegal_State
foo a -> Integer ! Illegal_State =
a+a
""",
uri.getAuthority())
.uri(uri)
.buildLiteral();
var module = ctx.eval(src);
var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
assertEquals(20, foo.execute(10).asInt());
try {
var res = foo.execute(".");
fail("Expecting an exception, not: " + res);
} catch (PolyglotException e) {
assertContains("expected the result of `foo` to be Integer, but got Text", e.getMessage());
}
}
/**
* The `!` part in the type signature is currently only for documentation purposes - it is not
* checked. Other kinds of dataflow errors may be returned as well and this is not an error. In
* particular, we don't distinguish errors raised by the function from errors propagate from its
* inputs.
*/
@Test
public void returnTypeCheckErrorSignatureAllowsAllErrors() throws Exception {
final URI uri = new URI("memory://rts.enso");
final Source src =
Source.newBuilder(
"enso",
"""
from Standard.Base import Integer, Error
import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
import Standard.Base.Errors.Illegal_State.Illegal_State
foo a -> Integer ! Illegal_State =
Error.throw (Illegal_Argument.Error "foo: "+a.to_text)
""",
uri.getAuthority())
.uri(uri)
.buildLiteral();
var module = ctx.eval(src);
var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
var result = foo.execute(10);
assertTrue(result.isException());
assertContains("Illegal_Argument.Error 'foo: 10'", result.toString());
}
@Test
public void returnTypeCheckSum() throws Exception {
final URI uri = new URI("memory://rts.enso");
final Source src =
Source.newBuilder(
"enso",
"""
from Standard.Base import Integer, Text
foo a -> Integer | Text =
a+a
""",
uri.getAuthority())
.uri(uri)
.buildLiteral();
var module = ctx.eval(src);
var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
assertEquals(20, foo.execute(10).asInt());
assertEquals("..", foo.execute(".").asString());
try {
Object vec = new Integer[] {1, 2, 3};
var res = foo.execute(vec);
fail("Expecting an exception, not: " + res);
} catch (PolyglotException e) {
assertContains(
"expected the result of `foo` to be Integer | Text, but got Vector", e.getMessage());
}
}
@Test
public void returnTypeCheckProduct() throws Exception {
final URI uri = new URI("memory://rts.enso");
final Source src =
Source.newBuilder(
"enso",
"""
from Standard.Base import Integer, Text
type Clazz
Value a
Clazz.from (that : Integer) = Clazz.Value that
foo a -> (Integer | Text) & Clazz =
a+a
""",
uri.getAuthority())
.uri(uri)
.buildLiteral();
var module = ctx.eval(src);
var foo = module.invokeMember(MethodNames.Module.EVAL_EXPRESSION, "foo");
assertEquals(20, foo.execute(10).asInt());
try {
var res = foo.execute(".");
fail("Expecting an exception, not: " + res);
} catch (PolyglotException e) {
assertContains(
"expected the result of `foo` to be (Integer | Text) & Clazz, but got Text",
e.getMessage());
}
}
static void assertTypeError(String expArg, String expType, String realType, String msg) { static void assertTypeError(String expArg, String expType, String realType, String msg) {
assertEquals( assertEquals(
"Type error: expected " + expArg + " to be " + expType + ", but got " + realType + ".", "Type error: expected " + expArg + " to be " + expType + ", but got " + realType + ".",

View File

@ -6,7 +6,6 @@ import org.enso.compiler.core.Implicits.AsMetadata
import org.enso.compiler.core.ir.Expression import org.enso.compiler.core.ir.Expression
import org.enso.compiler.core.ir.Function import org.enso.compiler.core.ir.Function
import org.enso.compiler.core.ir.Module import org.enso.compiler.core.ir.Module
import org.enso.compiler.core.ir.Literal
import org.enso.compiler.core.ir.expression.Application import org.enso.compiler.core.ir.expression.Application
import org.enso.compiler.core.ir.expression.errors import org.enso.compiler.core.ir.expression.errors
import org.enso.compiler.core.ir.module.scope.Definition import org.enso.compiler.core.ir.module.scope.Definition
@ -265,14 +264,24 @@ class TypeSignaturesTest extends CompilerTest {
"associate the signature with the typed expression" in { "associate the signature with the typed expression" in {
ir shouldBe an[Application.Prefix] ir shouldBe an[Application.Prefix]
ir.getMetadata(TypeSignatures) shouldBe defined val outerSignature = ir.getMetadata(TypeSignatures)
outerSignature shouldBe defined
outerSignature.get.signature.showCode() shouldEqual "Double"
} }
"work recursively" in { "work recursively" in {
val arg2Value = ir.asInstanceOf[Application.Prefix].arguments(1).value val arg2Value = ir.asInstanceOf[Application.Prefix].arguments(1).value
arg2Value shouldBe an[Application.Prefix] val arg2Signature = arg2Value.getMetadata(TypeSignatures)
val snd = arg2Value.asInstanceOf[Application.Prefix] arg2Signature shouldBe defined
snd.arguments(0).value shouldBe an[Literal.Number] arg2Signature.get.signature.showCode() shouldEqual "Int"
// But arg1 has no signature:
val arg1Signature = ir
.asInstanceOf[Application.Prefix]
.arguments(0)
.value
.getMetadata(TypeSignatures)
arg1Signature shouldBe empty
} }
} }
} }

View File

@ -82,6 +82,38 @@ public abstract class ReadArgumentCheckNode extends Node {
abstract String expectedTypeMessage(); abstract String expectedTypeMessage();
protected final String joinTypeParts(List<String> parts, String separator) {
assert !parts.isEmpty();
if (parts.size() == 1) {
return parts.get(0);
}
var separatorWithSpace = " " + separator + " ";
var builder = new StringBuilder();
boolean isFirst = true;
for (String part : parts) {
if (isFirst) {
isFirst = false;
} else {
builder.append(separatorWithSpace);
}
// If the part contains a space, it means it is not a single type but already a more complex
// expression with a separator.
// So to ensure we don't mess up the expression layers, we need to add parentheses around it.
boolean needsParentheses = part.contains(" ");
if (needsParentheses) {
builder.append("(");
}
builder.append(part);
if (needsParentheses) {
builder.append(")");
}
}
return builder.toString();
}
final PanicException panicAtTheEnd(Object v) { final PanicException panicAtTheEnd(Object v) {
if (expectedTypeMessage == null) { if (expectedTypeMessage == null) {
CompilerDirectives.transferToInterpreterAndInvalidate(); CompilerDirectives.transferToInterpreterAndInvalidate();
@ -195,9 +227,11 @@ public abstract class ReadArgumentCheckNode extends Node {
@Override @Override
String expectedTypeMessage() { String expectedTypeMessage() {
return Arrays.stream(checks) var parts =
.map(n -> n.expectedTypeMessage()) Arrays.stream(checks)
.collect(Collectors.joining(" & ")); .map(ReadArgumentCheckNode::expectedTypeMessage)
.collect(Collectors.toList());
return joinTypeParts(parts, "&");
} }
} }
@ -239,9 +273,11 @@ public abstract class ReadArgumentCheckNode extends Node {
@Override @Override
String expectedTypeMessage() { String expectedTypeMessage() {
return Arrays.stream(checks) var parts =
.map(n -> n.expectedTypeMessage()) Arrays.stream(checks)
.collect(Collectors.joining(" | ")); .map(ReadArgumentCheckNode::expectedTypeMessage)
.collect(Collectors.toList());
return joinTypeParts(parts, "|");
} }
} }

View File

@ -768,6 +768,12 @@ class IrToTruffle(
comment, comment,
context.getTopScope().getBuiltins().function() context.getTopScope().getBuiltins().function()
) )
case typeWithError: Tpe.Error =>
// When checking a `a ! b` type, we ignore the error part as it is only used for documentation purposes and is not checked.
extractAscribedType(comment, typeWithError.typed)
case typeInContext: Tpe.Context =>
// Type contexts aren't currently really used. But we should still check the base type.
extractAscribedType(comment, typeInContext.typed)
case t => { case t => {
t.getMetadata(TypeNames) match { t.getMetadata(TypeNames) match {
case Some( case Some(