Merge Small_Integer and Big_Integer types #7636

Akirathan · 2023-08-22T16:57:49Z

Pull Request Description

Merge Small_Integer and Big_Integer types into a single Integer type. Both Small_Integer and Big_Integer used to be the only internal types that were not visible in Enso, i.e., typing Small_Integer in REPL resulted in an identifier not found error.

Important Notes

Possible to call some methods on Integer as static ones, e.g., Integer.+, Integer.*, etc.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
All code follows the
Scala,
Java,
All code has been tested:
- Unit tests have been written where possible.

Akirathan · 2023-08-23T10:08:46Z

test/Tests/src/Data/Numbers_Spec.enso

+        Test.specify "should be able to invoke methods on Integer via static method call" <|
+            Integer.+ 1 2 . should_equal 3
+            Integer.+ 1 2.5 . should_equal 3.5
+            Test.expect_panic_with (Integer.+ 1.5 1) Type_Error


This results in Type_Error because self is 1.5 which is Decimal, and not Integer

…voked-statically

radeusgd · 2023-09-07T14:50:28Z

...time/src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/LessNode.java

  @Specialization
  boolean doDouble(EnsoBigInteger self, double that) {
    return BigIntegerOps.toDouble(self.getValue()) < that;


Shall we rename these to avoid overloads?

I'm not sure if this has any implications for the compilation, probably no.

But still, it feels cleaner and more consistent to have names be unique - AFAIK in other places all of our specializations do have unique names.

...ntime/src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/ModNode.java

radeusgd

Looks good to me. I'd like to see a test as mentioned in one of the comments.

JaroslavTulach

Conceptually OK. If tests are passing and benchmarks are sane, then let's integrate!

JaroslavTulach · 2023-09-08T05:09:41Z

...ntime/src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/AbsNode.java


-@BuiltinMethod(type = "Small_Integer", name = "abs", description = "Absolute value of a number")
+@BuiltinMethod(type = "Integer", name = "abs", description = "Absolute value of a number")
 public abstract class AbsNode extends Node {
  private @Child ToEnsoNumberNode toEnsoNumberNode = ToEnsoNumberNode.create();


We should probably make ToEnsoNumberNode inlineable. Otherwise all integer nodes will have two nodes - the main one and the ToEnsoNumberNode one.

JaroslavTulach · 2023-09-08T05:13:59Z

.../src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/IntegerUtils.java

+import org.enso.interpreter.runtime.error.PanicException;
+import org.enso.interpreter.runtime.library.dispatch.TypesLibrary;
+
+public final class IntegerUtils {


Consider merging BigIntegerOps and IntegerUtils into a single class.

JaroslavTulach · 2023-09-08T05:15:37Z

.../src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/IntegerUtils.java

+  }
+
+  @TruffleBoundary
+  static PanicException throwTypeErrorIfNotInt(Object self, Node node) {


Technically this doesn't have to be behind @TruffleBoundary - all of the operations can run on fast path. But maybe such errors are infrequent and we don't need them to run at full speed.

engine/runtime/src/test/java/org/enso/interpreter/test/IntegerTest.java

test/Tests/src/Semantic/Meta_Spec.enso

Akirathan · 2023-09-08T07:19:08Z

Engine benchmarks scheduled at https://github.com/enso-org/enso/actions/runs/6118815952, stdlib benches at https://github.com/enso-org/enso/actions/runs/6118827261

GitHub
Benchmark Engine · enso-org/enso@8a7208c
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

GitHub
Benchmark Standard Libraries · enso-org/enso@8a7208c
Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

Akirathan · 2023-09-08T14:04:17Z

Benchmark results are stable. Merging ....

Akirathan added 2 commits August 22, 2023 18:57

Merge Small_Integer.+ and Big_Integer.+

b19ee19

Fix typos in tests

f640465

Akirathan commented Aug 23, 2023

View reviewed changes

Akirathan added 10 commits September 5, 2023 09:44

Merge branch 'develop' into wip/akirathan/6959-Integer.+-cannot-be-in…

9d9730d

…voked-statically

Add integer node tests

1f8cbbe

Merge rest of Small_Integer and Big_Integer operation nodes to Integer

862889b

Fix PowNode

f0da5e7

Remove BigInteger and SmallInteger classes

1d3ee11

AddNode does not return java.math.BigInteger

795397a

Update CHANGELOG

54b1b71

Add missing specialization to AddNode

bfbef68

Fix error message in TextTest

619f8e1

Fix error message in RuntimeServerTest

19ee2f7

Akirathan self-assigned this Sep 6, 2023

Akirathan added 4 commits September 7, 2023 15:27

Fix method callsin RuntimeVisualizationsTest

6c26aac

Fix some shared cached params warnings

c4619f2

Clear up tests

f18b8a3

fmt

627dd60

Akirathan marked this pull request as ready for review September 7, 2023 14:20

Akirathan requested review from jdunkerley, radeusgd, GregoryTravis, 4e6, JaroslavTulach and hubertp as code owners September 7, 2023 14:20

Handle Type_Error

6adb207

Akirathan force-pushed the wip/akirathan/6959-Integer.+-cannot-be-invoked-statically branch from 70cd83f to 6adb207 Compare September 7, 2023 14:28

GregoryTravis approved these changes Sep 7, 2023

View reviewed changes

radeusgd reviewed Sep 7, 2023

View reviewed changes

...ntime/src/main/java/org/enso/interpreter/node/expression/builtin/number/integer/ModNode.java Outdated Show resolved Hide resolved

radeusgd approved these changes Sep 7, 2023

View reviewed changes

Akirathan added 4 commits September 7, 2023 17:31

Type_Error is wrapped in PanicException

6432e44

Fix module with big integer

bf475e1

Fix native image build

8bd0eef

Fix Meta_Spec - Integer type now has more methods

8a7208c

JaroslavTulach approved these changes Sep 8, 2023

View reviewed changes

Akirathan merged commit e0ee8fd into develop Sep 8, 2023
28 checks passed

Akirathan deleted the wip/akirathan/6959-Integer.+-cannot-be-invoked-statically branch September 8, 2023 14:04

enso-bot bot mentioned this pull request Sep 9, 2023

Remove \ from IR.Type.Set but support & #6165

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Merge Small_Integer and Big_Integer types #7636

Merge Small_Integer and Big_Integer types #7636

Akirathan commented Aug 22, 2023 •

edited

Loading

Akirathan Aug 23, 2023

radeusgd Sep 7, 2023

radeusgd left a comment

JaroslavTulach left a comment

JaroslavTulach Sep 8, 2023

JaroslavTulach Sep 8, 2023

JaroslavTulach Sep 8, 2023

Akirathan commented Sep 8, 2023 •

edited by unfurl-links bot

Loading

Akirathan commented Sep 8, 2023

Merge Small_Integer and Big_Integer types #7636

Merge Small_Integer and Big_Integer types #7636

Conversation

Akirathan commented Aug 22, 2023 • edited Loading

Pull Request Description

Important Notes

Checklist

Akirathan Aug 23, 2023

Choose a reason for hiding this comment

radeusgd Sep 7, 2023

Choose a reason for hiding this comment

radeusgd left a comment

Choose a reason for hiding this comment

JaroslavTulach left a comment

Choose a reason for hiding this comment

JaroslavTulach Sep 8, 2023

Choose a reason for hiding this comment

JaroslavTulach Sep 8, 2023

Choose a reason for hiding this comment

JaroslavTulach Sep 8, 2023

Choose a reason for hiding this comment

Akirathan commented Sep 8, 2023 • edited by unfurl-links bot Loading

Akirathan commented Sep 8, 2023

Akirathan commented Aug 22, 2023 •

edited

Loading

Akirathan commented Sep 8, 2023 •

edited by unfurl-links bot

Loading