Propagate warnings when calling `.length` on `Vector` by JaroslavTulach · Pull Request #14744 · enso-org/enso

JaroslavTulach · 2026-02-10T07:08:22Z

Pull Request Description

fixes Vector.length doesn't propagate vector Warning with the length value #11570
by following steps
keeping rest of the behavior consistent

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 not affected - 1st run
- second run after back outs
- 3rd run after a623bb3 - plus one more run
Library benchmarks are not affected - 1st run, another run

JaroslavTulach · 2026-02-10T10:31:19Z

There seems to be one test failure:

Use a slice of an array as vectors 114✅ 1❌
❌ Warnings and each
Expected False to be True (at /runner/_work/enso/enso/test/Base_Tests/src/Data/Vector_Spec.enso:1053:17-36).

Failure in JVM tests seems to be caused by outdated caches - e.g. not clean build

JaroslavTulach · 2026-02-10T11:58:58Z

Use a slice of an array as vectors 114✅ 1❌
❌ Warnings and each
Expected False to be True (at /runner/_work/enso/enso/test/Base_Tests/src/Data/Vector_Spec.enso:1053:17-36).

I had to disable the check in 9383295
the Array elements are unlikely to have warning attached
- the warning is assigned to the whole Array
- but elements are without warnings
not sure why it worked before...

JaroslavTulach · 2026-02-11T05:41:01Z

another failure related to each
VectorTest.insertSelfWithWarningViaForEach

...e/src/main/java/org/enso/interpreter/node/expression/builtin/immutable/LengthVectorNode.java

JaroslavTulach · 2026-02-16T06:55:33Z

Use a slice of an array as vectors 114✅ 1❌
❌ Warnings and each
Expected False to be True (at /runner/_work/enso/enso/test/Base_Tests/src/Data/Vector_Spec.enso:1053:17-36).
* I had to disable the check in [9383295](https://github.com/enso-org/enso/commit/93832952b6cc260838bf67ee5e0c22b4b9cdefb1)

* the `Array` elements are unlikely to have warning attached
  
  * the warning is assigned to the whole `Array`
  * but elements are without warnings

* not sure why it worked before...

there is still a failing test VectorTest.insertSelfWithWarningViaForEach
the problem is Array_Like_Helpers.each implementation and its treatment of warnings
the implementation calls vector.length and then uses it to iterate over 0.up_to len
- in the new version that strips the warnings
- and then reattached them later to the whole result
- making elements passed to each callback warning less
calling the old version of length (that doesn't touch warnings) in Array_Like_Helpers.each seems to "fix" the problem

JaroslavTulach · 2026-02-16T17:04:07Z

The problem with currently failing VectorTest is that InsertBuiltinVectorNode now produces WithWarnings[Vector.Generic] because length is associated with a warning!

…ents

…gVectorLength11570

JaroslavTulach · 2026-03-24T08:42:16Z

re-running failed jobs to find out how greenish this PR really is

…gVectorLength11570

…ings

JaroslavTulach · 2026-03-26T09:23:08Z

we seem to have a failure in Table_Tests when running
sbt 'runEngineDistribution --run test/Table_Tests should.allow.basic.operations'
after bisecting the failure seems to be caused by f476f85
now it feels like catch 22:
- sometimes (arr.each for example) we don't want to remove warnings from the elements)
- sometimes (when calling to Java) we want to remove the warnings from the elements
live would be so much easier, if we had Use asHostObject and simplify handling of WithWarnings #14833
- we could keep the warnings with elements
- they would be removed by asHostObject call when needed
backing f476f85 out and running test/Base_Tests works!?

Let's try to back `f476f85` out!

right now we have these failures
- 13118 passed, 53 failed and 287 skipped
- DuckDB_Tests/JUnit.xml 1542✅ 6❌
- Table_Tests/JUnit.xml 7509✅ 47❌
let's back out in b7cb93d (and in addition to that also b42645e) and see!

…ce over ArrayList

JaroslavTulach · 2026-03-26T10:40:00Z

distribution/lib/Standard/Base/0.0.0-dev/src/Data/Vector.enso

-        results_multiple.if_not_error (results_multiple.at 0)
+        res = results_multiple.if_not_error (results_multiple.at 0)
+        if propagate_warnings then res else
+            Warning.remove_warnings res


there is a lot of changes in this PR some kept, some backed out

with the assumption of the PR becoming greenish soon, here is my retrospective

the key fixed in this PR are:

Differentiate between length and raw_size

Vector.length doesn't propagate vector Warning with the length value #11570 requires length to carry out warnings

however for many internal usages this is not desirable

hence 665414a offers and uses internal raw_size method

Warning.remove_warning was Broken

it didn't work at all for Vector and Array

e.g. for anything else than WithWarnings wrapper

this got fixed in 1924195

Explicitly Removing Warnings

once the previous problem got fixed and removing warnings is reliable

it is now possible to explicitly remove warnings when propagate_warnings.not

done 58802e4

doing the removal explicitly at the right place seems to be the core of obtaining consistency

Intricate Construction of ArrayList

4cd21cb introduces a way to construct Array with elements with warnings

that provides consistent behavior of various Vector types in Vector_Spec

bringing more clarity into the test suite

JaroslavTulach · 2026-03-26T11:06:04Z

Benchmarking Engine

benchmarks from Mar 25 are not OK

let's see if today's "back outs" help to get them on par...
VectorBenchmarks_averageXyz made faster by a623bb3
3rd run scheduled and fixed the worst regression

JaroslavTulach · 2026-03-26T16:17:46Z

Benchmarking StdLib

1st run, 2nd run
most of the benchmarks aren't affected, but some are...
3rd run

JaroslavTulach · 2026-03-26T16:46:30Z

lib/java/benchmarks-common/src/main/java/org/enso/interpreter/bench/BenchmarksRunner.java

+    var add =
+        Stream.of(
+            "--foe=true",
+            "-jvmArgsAppend=--add-opens=org.graalvm.polyglot/org.graalvm.polyglot=ALL-UNNAMED");


With this change, it is possible to run a benchmark and easily polyglot profile it:

sbt:runtime-benchmarks> benchOnly VectorBenchmarks.averageOverArray

and meanwhile in VisualVM:

JaroslavTulach · 2026-03-26T17:25:10Z

.../runtime/src/main/java/org/enso/interpreter/node/expression/foreign/HostValueToEnsoNode.java


+  @Specialization
+  long doLong(long l) {
+    return l;


The VectorBenchmarks were slow because of expensive check for isNull:

Avoiding the check in a623bb3 for typical Enso values.

JaroslavTulach · 2026-03-30T12:05:41Z

Benchmarking StdLib

1st run, 2nd run

most of the benchmarks aren't affected, but some are...

3rd run

I see some improvements:

Overall the standard library results are OKeyish.

Propagate warnings when calling .length on Vector

1f71ffa

JaroslavTulach self-assigned this Feb 10, 2026

JaroslavTulach requested review from AdRiley, hubertp and jdunkerley as code owners February 10, 2026 07:08

JaroslavTulach added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 10, 2026

JaroslavTulach requested a review from 4e6 as a code owner February 10, 2026 07:08

JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Feb 10, 2026

Array elements are unlikely to have Warning attached

9383295

jdunkerley approved these changes Feb 11, 2026

View reviewed changes

4e6 approved these changes Feb 13, 2026

View reviewed changes

...e/src/main/java/org/enso/interpreter/node/expression/builtin/immutable/LengthVectorNode.java Outdated Show resolved Hide resolved

JaroslavTulach added 2 commits February 16, 2026 06:12

Rely on regular warnings manipulation

79834ee

Encapsulate these builtin classes

a32f6a5

JaroslavTulach added 3 commits February 16, 2026 17:12

map_with_index can be @Tail_Call function

396885e

Checking more combinations of map/each and containers

26d881f

We'd like this test to work also for slice

86a0dae

JaroslavTulach added 5 commits February 17, 2026 07:49

Inspect warnings on the container itself

11a1ddf

Use raw length without any attached warnings in Array_Like_Helpers

665414a

Dedicated assertHasWarning method

16ac8bc

Removing warnings from a Vector doesn't remove warnings from the elem…

f476f85

…ents

Merge remote-tracking branch 'origin/develop' into wip/jtulach/Warnin…

2343eac

…gVectorLength11570

JaroslavTulach added 3 commits March 25, 2026 15:12

Merge remote-tracking branch 'origin/develop' into wip/jtulach/Warnin…

b1b2884

…gVectorLength11570

Warning.remove_warning should really produce a clean_vec without warn…

1924195

…ings

Explicitly remove_warnings when propagate_warnings is False

58802e4

JaroslavTulach added 4 commits March 25, 2026 17:25

More documentation for Warning.set_array

ece8d18

Intricate way to create ArrayList with WithWarnings as elements

4cd21cb

Giving ArraySlice a withWarnings flag

64c5018

removeWarnings from compare result before classifying it

7f9a8bf

JaroslavTulach mentioned this pull request Mar 25, 2026

Vector.length doesn't propagate vector Warning with the length value #11570

Closed

Properly compute isLimitReached

fc7af11

JaroslavTulach added 2 commits March 26, 2026 10:33

Back f476f85 out

b7cb93d

Backing 64c5018 out and using more intricate way to construct the sli…

b42645e

…ce over ArrayList

JaroslavTulach commented Mar 26, 2026

View reviewed changes

Less checks in VectorTest as the new behavior was backed out

51ceb0e

Open up for VisualVM's polyglot sampler

af71400

JaroslavTulach commented Mar 26, 2026

View reviewed changes

Avoid isNull checks for typical Enso values

a623bb3

JaroslavTulach commented Mar 26, 2026

View reviewed changes

JaroslavTulach merged commit de9f00f into develop Mar 30, 2026
77 checks passed

JaroslavTulach deleted the wip/jtulach/WarningVectorLength11570 branch March 30, 2026 12:13

Conversation

JaroslavTulach commented Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Description

Checklist

Uh oh!

JaroslavTulach commented Feb 10, 2026

Uh oh!

JaroslavTulach commented Feb 10, 2026

Uh oh!

JaroslavTulach commented Feb 11, 2026

Uh oh!

Uh oh!

JaroslavTulach commented Feb 16, 2026

Uh oh!

JaroslavTulach commented Feb 16, 2026

Uh oh!

JaroslavTulach commented Mar 24, 2026

Uh oh!

JaroslavTulach commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Let's try to back f476f85 out!

Uh oh!

JaroslavTulach Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Differentiate between length and raw_size

Warning.remove_warning was Broken

Explicitly Removing Warnings

Intricate Construction of ArrayList

Uh oh!

JaroslavTulach commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarking Engine

Uh oh!

JaroslavTulach commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarking StdLib

Uh oh!

JaroslavTulach Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

JaroslavTulach Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

JaroslavTulach commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Benchmarking StdLib

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JaroslavTulach commented Feb 10, 2026 •

edited

Loading

JaroslavTulach commented Mar 26, 2026 •

edited

Loading

Let's try to back `f476f85` out!

JaroslavTulach Mar 26, 2026 •

edited

Loading

Differentiate between `length` and `raw_size`

`Warning.remove_warning` was Broken

Intricate Construction of `ArrayList`

JaroslavTulach commented Mar 26, 2026 •

edited

Loading

JaroslavTulach commented Mar 26, 2026 •

edited

Loading

JaroslavTulach commented Mar 30, 2026 •

edited

Loading