Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NaN is a new builtin type #5744

Closed
wants to merge 11 commits into from
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@
- [Moved regex functionality out of `Text.locate` and `Text.locate_all` into
`Text.match` and `Text.match_all`.][5679]
- [`File.parent` may return `Nothing`.][5699]
- [`Numbers.Nan` is a separate builtin type][5744]
- [Removed non-regex functionality from `is_match`, `match`, and `match_all`,
and renamed them to `match`, `find`, `find_all` (respectively).][5721]
- [Updated `rename_columns` to new API. Added `first_row`, `second_row` and
Expand Down Expand Up @@ -495,9 +496,10 @@
[5645]: https://github.com/enso-org/enso/pull/5645
[5646]: https://github.com/enso-org/enso/pull/5646
[5656]: https://github.com/enso-org/enso/pull/5656
[5678]: https://github.com/enso-org/enso/pull/5678
[5679]: https://github.com/enso-org/enso/pull/5679
[5699]: https://github.com/enso-org/enso/pull/5699
[5744]: https://github.com/enso-org/enso/pull/5744
[5678]: https://github.com/enso-org/enso/pull/5678
[5719]: https://github.com/enso-org/enso/pull/5719
[5721]: https://github.com/enso-org/enso/pull/5721

Expand Down
14 changes: 9 additions & 5 deletions distribution/lib/Standard/Base/0.0.0-dev/src/Data/Numbers.enso
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ type Number
negative_infinity = Double.NEGATIVE_INFINITY

## A constant holding the floating-point Not-a-Number value.
nan : Decimal
nan : Nan
nan = Double.NaN

## Checks if the given number is the floating-point Not-a-Number value.
Expand All @@ -348,6 +348,7 @@ type Number
compared with itself, so `x == Number.nan` would not work.
is_nan : Boolean
is_nan self = case self of
_ : Nan -> True
_ : Decimal -> Double.isNaN self
_ -> False

Expand Down Expand Up @@ -941,10 +942,8 @@ type Integer

parse_builtin text radix = @Builtin_Method "Integer.parse"

Comparable.from (that:Number) =
case that.is_nan of
True -> Incomparable
False -> Default_Ordered_Comparator
Comment on lines -944 to -947
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This used to be a huge performance bottleneck.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some speedup after the rewrite?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some speedup after the rewrite?

Yes. sieve.enso is 2x faster, and EqualsBenchmarks.equalsPrimitives is 10% faster. I have not tried other benchmarks, but I expect similar results, certainly no regressions.

@Builtin_Type
type Nan

## UNSTABLE

Expand All @@ -959,3 +958,8 @@ type Number_Parse_Error
to_display_text : Text
to_display_text =
"Could not parse " + self.text.to_text + " as a double."


Comparable.from (_:Integer) = Default_Ordered_Comparator
Comparable.from (_:Decimal) = Default_Ordered_Comparator
Comparable.from (_:Nan) = Incomparable
2 changes: 1 addition & 1 deletion distribution/lib/Standard/Base/0.0.0-dev/src/Main.enso
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export project.Warning.Warning

from project.Data.Boolean export Boolean, True, False
from project.Function export all
from project.Data.Numbers export Number, Integer, Decimal
from project.Data.Numbers export Number, Integer, Decimal, Nan
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
from project.System.File_Format export File_Format, Plain_Text_Format, Plain_Text, Bytes, Infer, Auto_Detect, JSON_File

import project.Data
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,11 @@ boolean doLongCheck(Type expectedType, long payload) {
@Specialization
boolean doDoubleCheck(Type expectedType, double payload) {
var numbers = EnsoContext.get(this).getBuiltins().number();
return checkParentTypes(numbers.getDecimal(), expectedType);
if (Double.isNaN(payload)) {
return checkParentTypes(numbers.getNan(), expectedType);
} else {
return checkParentTypes(numbers.getDecimal(), expectedType);
}
}

@Specialization
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,11 @@
import com.oracle.truffle.api.nodes.Node;
import org.enso.interpreter.dsl.AcceptsError;
import org.enso.interpreter.dsl.BuiltinMethod;
import org.enso.interpreter.epb.runtime.PolyglotExceptionProxy;
import org.enso.interpreter.epb.runtime.PolyglotProxy;
import org.enso.interpreter.runtime.EnsoContext;
import org.enso.interpreter.runtime.builtin.Builtins;
import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
import org.enso.interpreter.runtime.error.PanicException;
import org.enso.interpreter.runtime.error.PanicSentinel;
import org.enso.interpreter.runtime.error.Warning;
import org.enso.interpreter.runtime.error.WithWarnings;
import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
import org.enso.interpreter.runtime.number.EnsoBigInteger;
Expand All @@ -36,7 +33,12 @@ public static TypeOfNode build() {

@Specialization
Object doDouble(double value) {
return EnsoContext.get(this).getBuiltins().number().getDecimal();
var number = EnsoContext.get(this).getBuiltins().number();
if (Double.isNaN(value)) {
return number.getNan();
} else {
return number.getDecimal();
}
}

@Specialization
Expand Down Expand Up @@ -110,7 +112,15 @@ Object doPolyglotNumber(
if (interop.fitsInInt(proxy)) {
return builtins.number().getInteger();
} else if (interop.fitsInDouble(proxy)) {
return builtins.number().getDecimal();
try {
if (Double.isNaN(interop.asDouble(proxy))) {
return builtins.number().getNan();
} else {
return builtins.number().getDecimal();
}
} catch (UnsupportedMessageException e) {
throw new IllegalStateException(e);
}
} else {
return EnsoContext.get(this).getBuiltins().number();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package org.enso.interpreter.node.expression.builtin.number;

import org.enso.interpreter.dsl.BuiltinType;
import org.enso.interpreter.node.expression.builtin.Builtin;

@BuiltinType(name = "Standard.Base.Data.Numbers.Nan")
public class Nan extends Builtin {
@Override
protected Class<? extends Builtin> getSuperType() {
return Decimal.class;
}

@Override
protected boolean containsValues() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import org.enso.interpreter.node.expression.builtin.number.BigInteger;
import org.enso.interpreter.node.expression.builtin.number.Decimal;
import org.enso.interpreter.node.expression.builtin.number.Integer;
import org.enso.interpreter.node.expression.builtin.number.Nan;
import org.enso.interpreter.node.expression.builtin.number.SmallInteger;
import org.enso.interpreter.runtime.data.Type;

Expand All @@ -14,6 +15,7 @@ public class Number {
private final Builtin integer;
private final Builtin number;
private final Builtin decimal;
private final Builtin nan;

/** Creates builders for number Atom Constructors. */
public Number(Builtins builtins) {
Expand All @@ -23,6 +25,7 @@ public Number(Builtins builtins) {
number =
builtins.getBuiltinType(org.enso.interpreter.node.expression.builtin.number.Number.class);
decimal = builtins.getBuiltinType(Decimal.class);
nan = builtins.getBuiltinType(Nan.class);
}

/** @return the Int64 atom constructor. */
Expand All @@ -49,4 +52,8 @@ public Type getNumber() {
public Type getDecimal() {
return decimal.getType();
}

public Type getNan() {
return nan.getType();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ static boolean hasType(Double receiver) {

@ExportMessage
static Type getType(Double receiver, @CachedLibrary("receiver") TypesLibrary thisLib) {
return EnsoContext.get(thisLib).getBuiltins().number().getDecimal();
if (receiver.isNaN()) {
return EnsoContext.get(thisLib).getBuiltins().number().getNan();
} else {
return EnsoContext.get(thisLib).getBuiltins().number().getDecimal();
}
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package org.enso.interpreter.runtime.type;

import com.oracle.truffle.api.dsl.TypeCheck;
import com.oracle.truffle.api.dsl.TypeSystem;
import com.oracle.truffle.api.interop.ArityException;
import com.oracle.truffle.api.interop.UnsupportedTypeException;
import org.enso.interpreter.node.expression.builtin.number.Nan;
import org.enso.interpreter.runtime.callable.UnresolvedConversion;
import org.enso.interpreter.runtime.callable.UnresolvedSymbol;
import org.enso.interpreter.runtime.callable.atom.Atom;
Expand Down Expand Up @@ -30,6 +32,7 @@
long.class,
boolean.class,
double.class,
Nan.class,
Text.class,
Function.class,
Atom.class,
Expand Down Expand Up @@ -59,6 +62,16 @@
})
public class Types {

@TypeCheck(Nan.class)
public static boolean isNan(Object value) {
return value instanceof Double dbl && dbl.isNaN();
}

@TypeCheck(double.class)
public static boolean isDouble(Object value) {
return value instanceof Double dbl && !dbl.isNaN();
}

private static final TypeGraph typeHierarchy = buildTypeHierarchy();

/**
Expand Down Expand Up @@ -116,8 +129,12 @@ public static void extractArguments(Object[] arguments) throws ArityException {
public static String getName(Object value) {
if (TypesGen.isLong(value) || TypesGen.isEnsoBigInteger(value)) {
return ConstantsGen.INTEGER;
} else if (TypesGen.isDouble(value)) {
return ConstantsGen.DECIMAL;
} else if (value instanceof Double dbl) {
if (dbl.isNaN()) {
return ConstantsGen.NAN;
} else {
return ConstantsGen.DECIMAL;
}
} else if (TypesGen.isBoolean(value)) {
return ConstantsGen.BOOLEAN;
} else if (TypesGen.isText(value)) {
Expand Down Expand Up @@ -240,6 +257,7 @@ private static TypeGraph buildTypeHierarchy() {
graph.insert(ConstantsGen.ARRAY, ConstantsGen.ANY);
graph.insert(ConstantsGen.BOOLEAN, ConstantsGen.ANY);
graph.insert(ConstantsGen.DECIMAL, ConstantsGen.NUMBER);
graph.insert(ConstantsGen.NAN, ConstantsGen.DECIMAL);
graph.insert(ConstantsGen.ERROR, ConstantsGen.ANY);
graph.insert(ConstantsGen.FUNCTION, ConstantsGen.ANY);
graph.insert(ConstantsGen.INTEGER, ConstantsGen.NUMBER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,49 +25,29 @@ public class TypesFromProxy {
* @return the associated {@link Type} if it exists and {@code null} otherwise
*/
public static Type fromTypeSystem(Builtins builtins, String typeName) {
switch (typeName) {
case ConstantsGen.ANY:
return builtins.any();
case ConstantsGen.ARRAY:
return builtins.array();
case ConstantsGen.BOOLEAN:
return builtins.bool().getType();
case ConstantsGen.DATE:
return builtins.date();
case ConstantsGen.DATE_TIME:
return builtins.dateTime();
case ConstantsGen.DURATION:
return builtins.duration();
case ConstantsGen.DECIMAL:
return builtins.number().getDecimal();
case ConstantsGen.ERROR:
return builtins.dataflowError();
case ConstantsGen.FUNCTION:
return builtins.function();
case ConstantsGen.FILE:
return builtins.file();
case ConstantsGen.INTEGER:
return builtins.number().getInteger();
case ConstantsGen.MANAGED_RESOURCE:
return builtins.managedResource();
case ConstantsGen.NOTHING:
return builtins.nothing();
case ConstantsGen.NUMBER:
return builtins.number().getNumber();
case ConstantsGen.PANIC:
return builtins.panic();
case ConstantsGen.REF:
return builtins.ref();
case ConstantsGen.TEXT:
return builtins.text();
case ConstantsGen.TIME_OF_DAY:
return builtins.timeOfDay();
case ConstantsGen.TIME_ZONE:
return builtins.timeZone();
case ConstantsGen.VECTOR:
return builtins.vector();
default:
return null;
}
return switch (typeName) {
case ConstantsGen.ANY -> builtins.any();
Akirathan marked this conversation as resolved.
Show resolved Hide resolved
case ConstantsGen.ARRAY -> builtins.array();
case ConstantsGen.BOOLEAN -> builtins.bool().getType();
case ConstantsGen.DATE -> builtins.date();
case ConstantsGen.DATE_TIME -> builtins.dateTime();
case ConstantsGen.DURATION -> builtins.duration();
case ConstantsGen.DECIMAL -> builtins.number().getDecimal();
case ConstantsGen.NAN -> builtins.number().getNan();
case ConstantsGen.ERROR -> builtins.dataflowError();
case ConstantsGen.FUNCTION -> builtins.function();
case ConstantsGen.FILE -> builtins.file();
case ConstantsGen.INTEGER -> builtins.number().getInteger();
case ConstantsGen.MANAGED_RESOURCE -> builtins.managedResource();
case ConstantsGen.NOTHING -> builtins.nothing();
case ConstantsGen.NUMBER -> builtins.number().getNumber();
case ConstantsGen.PANIC -> builtins.panic();
case ConstantsGen.REF -> builtins.ref();
case ConstantsGen.TEXT -> builtins.text();
case ConstantsGen.TIME_OF_DAY -> builtins.timeOfDay();
case ConstantsGen.TIME_ZONE -> builtins.timeZone();
case ConstantsGen.VECTOR -> builtins.vector();
default -> null;
};
}
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,21 @@
package org.enso.interpreter.test;

import java.io.ByteArrayOutputStream;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.net.URI;
import java.nio.file.Paths;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import org.enso.interpreter.runtime.type.ConstantsGen;
import org.enso.polyglot.RuntimeOptions;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine;
import org.graalvm.polyglot.Language;
import org.graalvm.polyglot.Source;
import org.graalvm.polyglot.Value;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -148,8 +142,13 @@ private void checkAllTypesSatisfy(Check check) throws Exception {
continue;
}
switch (t.getMetaSimpleName()) {
// represented as primitive values without meta object
// represented as primitive values without meta objects
case "Number" -> {}
case "Decimal" -> {}
JaroslavTulach marked this conversation as resolved.
Show resolved Hide resolved
case "Integer" -> {}
case "Small_Integer" -> {}
case "Big_Integer" -> {}
case "Nan" -> {}
// has no instances
case "Array_Proxy" -> {}
// Warning is transparent and invisible
Expand Down
Loading