All Enso objects are hasheable

[Akirathan]

Pull Request Description

Implement hash_code for all Enso objects. Map is now a builtin type with a proper hash map implementation. This PR is a follow-up of Convert Any.== to a builtin PR.

Important Notes

• Replaces binary tree Map implementation with proper hash map implementation.
• There are almost no modifications to the API of Map, except for:
  • to_vector no longer returns vector sorted by keys
  • keys returns unsorted vector
  • first and last methods are removed
• Map supports any object as keys - primitives, atoms, polyglot values, another maps, ...
• Foreign maps (Python dictionary, JavaScript Map, java.util.Map, etc.) are treated as Enso maps seamlessly, just how foreign arrays are treated.
• Add tests that ensure that the following holds:
  • For all objects o1, o2: if o1 == o2 then hash(o1) == hash(o2) and if hash(o1) != hash(o2) then o1 != o2.
• Added many values to ValueGenerator.

Checklist

Please include the following checklist in your PR:

• The documentation has been updated if necessary.
• All code conforms to the
  Scala,
  Java,
  and
  Rust
  style guides.
• All code has been tested:
  • Unit tests have been written where possible.
  • If GUI codebase was changed: Enso GUI was tested when built using BOTH
    ./run ide build and ./run ide watch.

[JaroslavTulach]

Thank you for the proposal, Pavel! Finally something material to base our discussion on.

One of my suggestions is to separate API from SPI. In Java hashCode is both - something to call & something to override - that's wrong as it doesn't allow the language/runtime enforce any behavior of the SPI. I'd like to avoid that and thus I am proposing to remove Any.hash_code.

However, according to @kustosz, we want people to override Any.== as well as hash_code in some situations. Marcin suggested to have compute_hash_code method - OK, that would mean - if a Type finds there is compute_hash_code method defined in it, then it calls it from Meta.hash_code rather than computing the default value.

In both cases the once computed hash_code shall be cached in the Atom.

I suggest to get more feedback from involved parties and then evolve the proposal a bit. In any case thanks a lot for starting this PR. It is great to see real (hash) code!

[JaroslavTulach]

Based on the feedback in this PR I prepared an inception review document please take a look and modify it/add comments here, etc. My current feeling is that we could split the work into three (or even more) independent PRs and deliver this piece by piece.

[Akirathan]

@JaroslavTulach FYI: I modified the PR so that it is more in line with the current idea of hashing - added Hash_Map type and improved hash code caching. It is still very much work in progress, but the following simple snippet already kind of works:

from Standard.Base.Data.Hash_Map import Hash_Map, Builder

main =
    builder = Builder.new
    builder.append "A" 1
    hash_map = builder.build
    IO.println (hash_map.get "A")

[JaroslavTulach]

There is a windows engine failure probably related to ordering of keys in a map:

- should allow converting a GeoJSON object into a table with provided fields [106ms]
- [FAILED] should allow converting a GeoJSON object into a table containing all available fields [137ms]
    Reason: 
    [longitude, latitude, foo, bar, elevation, baz]
    did not equal 
    [bar, baz, elevation, foo, latitude, longitude] 
(at C:\runner\_work\enso\enso\test\Geo_Tests\src\Geo_Spec.enso:53:13-53).

[JaroslavTulach]

There is a failure in Visualization tests - probably we can modify the test to accept the values in any order.

- [FAILED] limit the number of squared elements [57ms]
   Reason: [{"x":0,"y":225}, {"x":29,"y":196}, {"x":15,"y":0}] did not equal [{"x":0,"y":225}, {"x":15,"y":0}, {"x":29,"y"

[JaroslavTulach]

Failures in Table tests:

Reason: An unexpected dataflow error (There was an SQL error: FATAL: role "mwu" does not exist.) 
has been matched (at /runner/_work/enso/enso/test/Table_Tests/src/Database/Common_Spec.enso:37:13-49).

Probably not related to Pavel's code, but to some infrastructure problem. CCing @mwu-tow.

[Akirathan]

Failures in Table tests:

Reason: An unexpected dataflow error (There was an SQL error: FATAL: role "mwu" does not exist.) 
has been matched (at /runner/_work/enso/enso/test/Table_Tests/src/Database/Common_Spec.enso:37:13-49).

Probably not related to Pavel's code, but to some infrastructure problem. CCing @mwu-tow.

It could be related to maps. It is failing in Postgres_Spec.enso, where, I think, java.util.Properties are used for the configurations, and they are treated as Maps. Not only that, but it is possible that wrong Properties are passed because of changes in this PR. Right now, I am trying to reproduce this locally, but I haven't even installed Docker, so it will take a while. Besides, I have already quickly checked whether this is an infra issue and other Engine CI jobs are not failing with this.

[mwu-tow]

@JaroslavTulach @Akirathan
Strange but I don't think it is CI-caused. I don't know where the mwu comes from, it should not be used in the postgresql server configuration. It is however the username, so if you get it anywhere from the environment, it might show up. Are the test now respecting the ENSO_DATABASE_TEST_DB_USER variable?

[Akirathan]

@JaroslavTulach @Akirathan Strange but I don't think it is CI-caused. I don't know where the mwu comes from, it should not be used in the postgresql server configuration. It is however the username, so if you get it anywhere from the environment, it might show up. Are the test now respecting the ENSO_DATABASE_TEST_DB_USER variable?

Turns out that this issue is, indeed, caused by my changes. I tried logging how I manipulate with java.util.Properties that are used to initialize the connection here https://github.com/enso-org/enso/actions/runs/3951865906/jobs/6766255048#step:10:11400 which clearly proves that the transformation is wrong. I will try to debug that locally. Luckily, I don't need to initialize Postgres for that.

GitHub

All Enso objects are hasheable · enso-org/enso@8b7979b

Hybrid visual and textual functional programming. Contribute to enso-org/enso development by creating an account on GitHub.

[JaroslavTulach]

Last(?) failure on windows:

- [FAILED] should insert entries to a polyglot map [23ms]
Reason: An unexpected panic was thrown: ForeignParsingException: 
'Cannot parse foreign python method. Only available languages are [enso, js]'

you need to shield the python tests by a check for a presence of that language.

[JaroslavTulach]

I am glad the Polyglot.invoke works and workarounds the perception of java.util.Properties being a Map.