Unused arguments cause buildEngineDistribution to generate invalid caches and prevent tests requiring the affected file from running

[radeusgd]

I have noticed a bug with serialization of org.enso.compiler.core.ir.expression.warnings.Unused$FunctionArgument. At first it looked innocuous - just preventing some IR caches from loading - but of course it should just then parse the original source and all should be fine.

Well - unfortunately I have found that for some reason instead of doing that and recovering - in some scenarios (described below) the invalid cache causes the program to fail altogether.

The repro is following:

1. Checkout develop and buildEngineDistribution in sbt.
2. Add a function foo bar = 42 at the end of File.enso - the bar argument is unused causing a warning. See patch at the bottom.
3. Again, do buildEngineDistribution.
4. Run .\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\bin\enso --run .\test\Tests\src\System\File_Spec.enso.

The (4) run fails with:

C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso: error: The symbol Existing_File_Behavior (module, type, or constructor) does not exist in File.
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso: error: The symbol File_Access (module, type, or constructor) does not exist in File.
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso: error: The symbol File_Permissions (module, type, or constructor) does not exist in File.
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso: error: The symbol Write_Extensions (module, type, or constructor) does not exist in File.
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:47:14: error: The name `Vector` could not be found.
   47 | file_types : Vector
      |              ^~~~~~

(...)

C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:802:39: error: The name `Nothing` could not be found.
  802 |         if file.is_directory.not then Nothing else
      |                                       ^~~~~~~
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:804:40: error: The name `Option` could not be found.
  804 |             options = children.map c-> Option c.name c.name.pretty
      |                                        ^~~~~~
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:805:13: error: The name `Widget` could not be found.
  805 |             Widget.Single_Choice values=options display=Display.Always
      |             ^~~~~~
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:805:57: error: The name `Display` could not be found.
  805 |             Widget.Single_Choice values=options display=Display.Always
      |                                                         ^~~~~~~
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:807:5: warning: Unused function argument bar.
  807 | foo bar = 42
      |     ^~~
Aborting due to 186 errors and 1 warnings.
Execution finished with an error: Compilation aborted due to errors.

We can see that constructors like Nothing are not resolved correctly - but they are obviously imported at the top of the File.enso...

Apparently, adding the unused argument has broken the whole file.

For context, this is what is written out when running the second buildEngineDistribution:

(...)

[info] Assembly jar up to date: runner.jar
[info] Regenerating manifest for [distribution\lib\Standard\Base\0.0.0-dev].
[info] Generating index for built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base
C:\NBO\enso\built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev\lib\Standard\Base\0.0.0-dev\src\System\File.enso:807:5: warning: Unused function argument bar.
  807 | foo bar = 42
      |     ^~~
[ERROR] [2023-11-24T13:29:49+01:00] [enso.org.enso.interpreter.caches.ModuleCache] Failed to save cache for Standard.Base.System.File.
java.io.IOException: No persistance for org.enso.compiler.core.ir.expression.warnings.Unused$FunctionArgument
        at org.enso.runtime/org.enso.persist.PerMap.searchSupertype(PerMap.java:63)
        at org.enso.runtime/org.enso.persist.PerMap.searchSupertype(PerMap.java:56)
        at org.enso.runtime/org.enso.persist.PerMap.searchSupertype(PerMap.java:56)
        at org.enso.runtime/org.enso.persist.PerMap.forType(PerMap.java:73)
        at org.enso.runtime/org.enso.persist.PerGenerator.writeIndirect(PerGenerator.java:67)
        at org.enso.runtime/org.enso.persist.PerGenerator$ReferenceOutput.writeObject(PerGenerator.java:104)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistScalaList.writeObject(IrPersistance.java:192)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistScalaList.writeObject(IrPersistance.java:180)
        at org.enso.runtime/org.enso.persist.Persistance.writeInline(Persistance.java:139)
        at org.enso.runtime/org.enso.persist.PerGenerator$ReferenceOutput.writeInline(PerGenerator.java:99)
        at org.enso.runtime/org.enso.compiler.pass.analyse.PersistGatherDiagnostics_DiagnosticsMeta.writeObject(PersistGatherDiagnostics_DiagnosticsMeta.java:18)
        at org.enso.runtime/org.enso.compiler.pass.analyse.PersistGatherDiagnostics_DiagnosticsMeta.writeObject(PersistGatherDiagnostics_DiagnosticsMeta.java:4)
        at org.enso.runtime/org.enso.persist.Persistance.writeInline(Persistance.java:139)
        at org.enso.runtime/org.enso.persist.PerGenerator.writeIndirect(PerGenerator.java:72)
        at org.enso.runtime/org.enso.persist.PerGenerator$ReferenceOutput.writeObject(PerGenerator.java:104)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistScalaMap.writeObject(IrPersistance.java:225)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistScalaMap.writeObject(IrPersistance.java:210)
        at org.enso.runtime/org.enso.persist.Persistance.writeInline(Persistance.java:139)
        at org.enso.runtime/org.enso.persist.PerGenerator$ReferenceOutput.writeInline(PerGenerator.java:99)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistMetadataStorage.writeObject(IrPersistance.java:385)
        at org.enso.runtime/org.enso.compiler.core.ir.IrPersistance$PersistMetadataStorage.writeObject(IrPersistance.java:370)
        at org.enso.runtime/org.enso.persist.Persistance.writeInline(Persistance.java:139)
        at org.enso.runtime/org.enso.persist.PerGenerator$ReferenceOutput.writeInline(PerGenerator.java:99)
        at org.enso.runtime/org.enso.compiler.core.ir.PersistModule.writeObject(PersistModule.java:29)
        at org.enso.runtime/org.enso.compiler.core.ir.PersistModule.writeObject(PersistModule.java:4)
        at org.enso.runtime/org.enso.persist.Persistance.writeInline(Persistance.java:139)
        at org.enso.runtime/org.enso.persist.PerGenerator.writeObject(PerGenerator.java:51)
        at org.enso.runtime/org.enso.persist.PerGenerator.writeObject(PerGenerator.java:33)
        at org.enso.runtime/org.enso.persist.Persistance.write(Persistance.java:173)
        at org.enso.runtime/org.enso.interpreter.caches.ModuleCache.serialize(ModuleCache.java:147)
        at org.enso.runtime/org.enso.interpreter.caches.ModuleCache.serialize(ModuleCache.java:26)
        at org.enso.runtime/org.enso.interpreter.caches.Cache.saveCacheTo(Cache.java:127)
        at org.enso.runtime/org.enso.interpreter.caches.Cache.lambda$save$0(Cache.java:97)
        at java.base/java.util.Optional.flatMap(Optional.java:289)
        at org.enso.runtime/org.enso.interpreter.caches.Cache.save(Cache.java:85)
        at org.enso.runtime/org.enso.interpreter.runtime.TruffleCompilerContext.saveCache(TruffleCompilerContext.java:186)
        at org.enso.runtime/org.enso.interpreter.runtime.SerializationManager.$anonfun$doSerializeModule$1(SerializationManager.scala:611)
        at org.enso.runtime/org.enso.interpreter.runtime.SerializationManager.$anonfun$doSerializeModule$1$adapted(SerializationManager.scala:591)
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
        at java.base/java.lang.Thread.run(Thread.java:1583)
        at org.graalvm.truffle/com.oracle.truffle.polyglot.SystemThread.run(SystemThread.java:68)
[info] Engine package created at built-distribution\enso-engine-0.0.0-dev-windows-amd64\enso-0.0.0-dev
[success] Total time: 67 s (01:07), completed 24 lis 2023, 13:29:51

---

The patch for `File.enso`

Index: distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso b/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso
--- a/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso	(revision b33285c634984d9270ab05e35c94f1abe3963899)
+++ b/distribution/lib/Standard/Base/0.0.0-dev/src/System/File.enso	(date 1700828913032)
@@ -803,3 +803,5 @@
             children = file.list
             options = children.map c-> Option c.name c.name.pretty
             Widget.Single_Choice values=options display=Display.Always
+
+foo bar = 42

[radeusgd]

The problem with this bug is that merely adding a function with unused argument which is just a warning completely prevents the program from running after the indexes are rebuilt by buildEngineDistribution.

The 'workaround' is to ensure there are no unused variables in std-lib (which is of course the desired state for production anyway) - but during development it is a bit problematic as sometimes one may want to test a partially implemented solution.

---

The biggest issue IMO is not that a warning is not serializable (although ideally it should be), but that somehow our caches get into a state in which instead of invalidating them and re-parsing the file, they are loaded but they are malformed and the program fails.

This is actually similar to intermittent IR caches issues I've been seeing from time to time - but I was never able to reliably reproduce that. This time it seems that the repro is completely deterministic.

Before fixing the missing serialization logic, it may be worth to investigate what happens so that the malformed cache is not invalidated - as that could potentially also affect the other, more intermittent, failures that we cannot easily reproduce.

[JaroslavTulach]

Thanks for the reproducer. I'll take a look.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-26):

Progress: - analyzing benchmarks: #8390 (comment)

• benchmarking filter PR: Filter for test/Benchmarks #8391
• 360 feedback It should be finished by 2023-11-30.

Next Day: Unused argument fixes

[JaroslavTulach]

The problem is caused by Base.bindings cache being stored, but File.ir cache not. One possible fix would be to avoid storing .bindings cache when any of its related .ir files is corrupted. Still investigating why the missing File.ir is causing the import resolution to fail.

[radeusgd]

One possible fix would be to avoid storing .bindings cache when any of its related .ir files is corrupted.

Should we instead invalidate the .bindings cache on load? I guess not storing it is also good, but even if it is stored but the related .ir file is missing (e.g. someone deleted the .ir file, but did not delete the .bindings file), I guess the program should still work just fine (probably just 'eagerly' invalidating all caches that may be only 'partially valid').

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-27):

Progress: - unused argument: #8384
- PR with a bit of robustness: #8393
- missing File.ir is the problem: #8384 (comment)
- playing with immutable BindingsMap
- merging debug doc unification: #8370
- integrated 3% speedup: #8359
- test updateModule: 7ab2ed2
- trying various ways to speed startup
- MetadataStorage.java: #8366
- meetings, standups, demo It should be finished by 2023-11-30.

Next Day: Unused argument fixes

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-28):

Progress: - More robust caches integrated: #8393

• decimal type thought: Provide engine support for decimal & co. types #8346 (comment)
• writing test for Chained operator block syntax #7904 It should be finished by 2023-11-30.

Next Day: chained operator blocks.

[enso-bot]

Jaroslav Tulach reports a new STANDUP for yesterday (2023-11-29):

Progress: - fixed #7904 by #8415

• fixed Java constants do not match correctly in case patterns #7964 - case on constants PR integrated: Convert constants to Enso values #8416
• question about operator precedence: https://github.com/enso-org/enso/pull/8415/files#r1408888819
• digging thru heap dump from RuntimeVisualizationTest
• meetings and standups
• Map.insert PR: 400x faster with linear hashing of the hash map entries #8425 It should be finished by 2023-11-30.

Next Day: Map.insert

GitHub

Multi line chained operator syntax by JaroslavTulach · Pull Request #8415 · enso-org/enso

Pull Request Description Fixes #7904. Checklist Please ensure that the following checklist has been satisfied before submitting the PR:

All code follows the
Java,
style guides.
All code has been ...

[enso-bot]

Jaroslav Tulach reports a new STANDUP for today (2023-11-30):

Progress: - sampling the hashmaps on JDK17 & IGV graphs checking - looks OKeyish

• bugfixed failures, ready for review: 400x faster with linear hashing of the hash map entries #8425
• fixing failures in Table_Tests: 67bc16e
• reporting VisualVM polyglot Sampler bug: Polyglot Sampler doesn't work with GraalVM for JDK 21 oracle/visualvm#537 It should be finished by 2023-11-30.

Next Day: Integrate my PRs