Propagate comparator warnings via Any.==

[JaroslavTulach]

Pull Request Description

Fixes #10679 by changing the return type of EqualsXyzNodes to EqualsAndInfo. This class holds the result of the comparation as well as any attached warnings. EqualsBuiltinNode then re-attaches the warnings, if there are any.

Checklist

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

• All code follows the
  Scala,
  Java,
• Unit tests have been written where possible.
• Engine benchmarks are fine: run, another run, run after merge - run after fix with 2c4b522
• Library benchmarks seem fine: run, another run and after fix run

[GregoryTravis]

These tests should pass now.

[JaroslavTulach]

There is 21 failing tests in Table_Tests what do you think about them, @GregoryTravis?

[GregoryTravis]

There is 21 failing tests in Table_Tests what do you think about them, @GregoryTravis?

The failures say first difference at index Nothing, which is an error I have not seen before.

[JaroslavTulach]

These tests should pass now.

These tests pass after 445e500

[JaroslavTulach]

Engine benchmarks run is finished. Here are potential problems:

• AtomBenchmarks_benchGenerateListQualified slightly slower
• IfVsCaseBenchmarks_caseBench3 - improved!
• RecursionBenchmarks_benchNestedThunkSum slightly slower
• RecursionBenchmarks_benchSumStateTCO slighlty slower
• TypePatternBenchmarks_matchOverAny minimal regression
• TypePatternBenchmarks_matchOverDecimal minimal regression
• etc.

Update on Sep 25, 2024: The benchmarks regression should now be eliminated with 2c4b522 to be verified by engine benchmarks run.

[Akirathan]

I don't like the introduction of the new EqualsAndInfo return type. Why can't EqualsNode.execute just return Object and for some specializations that should return a result with warnings just return WithWarnings object, and for other a boxed Boolean? Introduction of EqualsAndInfo adds an inconsistency to how we manipulate with warnings.

[jdunkerley]

Approved the enso tests to allow it to proceed.

[JaroslavTulach]

I don't like the introduction of the new EqualsAndInfo return type.
Why can't EqualsNode.execute just return Object and for some specializations that should return a result with warnings just return WithWarnings object, and for other a boxed Boolean?

Such an approach has been tried already. Please see this diff: #10679 (comment) - there are few problems to it:

Checking the result becomes more complicated. The above patch is using instanceof Boolean, but that's wrong. The code would probably deserve its own nodes: CheckBooleanNode and RewrapWarningFromBooleanNode, etc. Given the majority of use-cases (EqualsBuiltinNode``HashMap and comparator) are built around really need the boolean use-case it seems inefficient to wrap and immediately unwrap the value.

Warnings aren't efficient. Given 99.9% of use-cases is going to work with boolean only, having all the machinery in place for warnings manipulation is an over kill. It should be conditional. The current PR does that by checking equals() and making a decision. Checking warnings() aren't null and rewriting itself to handle the complicated case. No WarningsLibrary, no AppendWarningNode until a boolean with warning appears.

Introduction of EqualsAndInfo adds an inconsistency to how we manipulate with warnings.

A bit. But EqualsNode is special. It is not an expression! We have EqualsBuiltinNode now to represent an expression. EqualsNode is there to check == and give you EqualsAndInfo - EqualsBuiltinNode than extracts such info and converts it into regular EnsoObject.

I don't expect too much code in Enso to use EqualsNode directly. The EqualsBuiltinNode is one. The HashMap package is the other. The comparator is the 3rd. Maybe we can move the EqualsNode "family" into own package to demonstrate it is "special"?

[JaroslavTulach]

Benchmarks seem to be fine. One good result on bankers, @GregoryTravis: