Enable asserts in the tests (#4074)

Enso unit tests were running without `-ea` check enabled and as such various invariant checks in Truffle code were not executed. Let's turn the `-ea` flag on and fix all the code misbehaves.
enso-org · Jan 26, 2023 · ca2f108 · ca2f108
1 parent ad6419f
commit ca2f108
Show file tree

Hide file tree

Showing 25 changed files with 292 additions and 115 deletions.
diff --git a/build.sbt b/build.sbt
@@ -1106,7 +1106,7 @@ lazy val `interpreter-dsl` = (project in file("lib/scala/interpreter-dsl"))
 // === Sub-Projects ===========================================================
 // ============================================================================
 
-val truffleRunOptions = if (java.lang.Boolean.getBoolean("bench.compileOnly")) {
+val truffleRunOptionsNoAssert = if (java.lang.Boolean.getBoolean("bench.compileOnly")) {
   Seq(
     "-Dpolyglot.engine.IterativePartialEscape=true",
     "-Dpolyglot.engine.BackgroundCompilation=false",
@@ -1118,6 +1118,12 @@ val truffleRunOptions = if (java.lang.Boolean.getBoolean("bench.compileOnly")) {
     "-Dpolyglot.engine.BackgroundCompilation=false"
   )
 }
+val truffleRunOptions = "-ea" +: truffleRunOptionsNoAssert
+
+val truffleRunOptionsNoAssertSettings = Seq(
+  fork := true,
+  javaOptions ++= truffleRunOptionsNoAssert
+)
 
 val truffleRunOptionsSettings = Seq(
   fork := true,
@@ -1127,6 +1133,7 @@ val truffleRunOptionsSettings = Seq(
 lazy val `polyglot-api` = project
   .in(file("engine/polyglot-api"))
   .settings(
+    frgaalJavaCompilerSetting,
     Test / fork := true,
     commands += WithDebugCommand.withDebug,
     Test / envVars ++= distributionEnvironmentOverrides,
@@ -1180,6 +1187,7 @@ lazy val `language-server` = (project in file("engine/language-server"))
   )
   .configs(Benchmark)
   .settings(
+    inConfig(Compile)(truffleRunOptionsSettings),
     inConfig(Benchmark)(Defaults.testSettings),
     bench := (Benchmark / test).value,
     libraryDependencies += "com.storm-enroute" %% "scalameter" % scalameterVersion % "bench",
@@ -1575,7 +1583,7 @@ lazy val `runtime-with-polyglot` =
     .configs(Benchmark)
     .settings(
       frgaalJavaCompilerSetting,
-      inConfig(Compile)(truffleRunOptionsSettings),
+      inConfig(Compile)(truffleRunOptionsNoAssertSettings),
       inConfig(Benchmark)(Defaults.testSettings),
       commands += WithDebugCommand.withDebug,
       Benchmark / javacOptions --= Seq(

diff --git a/...nts/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java b/...nts/src/test/java/org/enso/interpreter/test/instrument/AvoidIdInstrumentationTagTest.java
@@ -1,11 +1,9 @@
 package org.enso.interpreter.test.instrument;
 import com.oracle.truffle.api.instrumentation.InstrumentableNode;
 import com.oracle.truffle.api.instrumentation.StandardTags;
-import com.oracle.truffle.api.nodes.Node;
 import com.oracle.truffle.api.source.SourceSection;
 import java.io.OutputStream;
 import java.nio.file.Paths;
-import java.util.List;
 import java.util.Map;
 import java.util.function.Predicate;
 import org.enso.interpreter.node.ClosureRootNode;
@@ -14,39 +12,36 @@
 import org.enso.interpreter.test.NodeCountingTestInstrument;
 import org.enso.polyglot.RuntimeOptions;
 import org.graalvm.polyglot.Context;
-import org.graalvm.polyglot.Engine;
 import org.graalvm.polyglot.Language;
+import org.graalvm.polyglot.Source;
 import org.junit.After;
 import org.junit.Assert;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.fail;
 import org.junit.Before;
 import org.junit.Test;
 
 public class AvoidIdInstrumentationTagTest {
 
-  private Engine engine;
   private Context context;
   private NodeCountingTestInstrument nodes;
 
   @Before
   public void initContext() {
-    engine = Engine.newBuilder()
+    context = Context.newBuilder()
         .allowExperimentalOptions(true)
         .option(
             RuntimeOptions.LANGUAGE_HOME_OVERRIDE,
             Paths.get("../../distribution/component").toFile().getAbsolutePath()
         )
         .logHandler(OutputStream.nullOutputStream())
-        .build();
-
-    context = Context.newBuilder()
-        .engine(engine)
         .allowExperimentalOptions(true)
         .allowIO(true)
         .allowAllAccess(true)
         .build();
 
+    var engine = context.getEngine();
     Map<String, Language> langs = engine.getLanguages();
     Assert.assertNotNull("Enso found: " + langs, langs.get("enso"));
 
@@ -57,26 +52,26 @@ public void initContext() {
   @After
   public void disposeContext() {
     context.close();
-    engine.close();
   }
 
   @Test
-  public void avoidIdInstrumentationInLambdaMapFunctionWithNoise() {
+  public void avoidIdInstrumentationInLambdaMapFunctionWithNoise() throws Exception {
     var code = """
     from Standard.Base import all
     import Standard.Visualization
 
     run n = 0.up_to n . map i-> 1.noise * i
     """;
-
-    var module = context.eval("enso", code);
+    var src = Source.newBuilder("enso", code, "TestLambda.enso").build();
+    var module = context.eval(src);
     var run = module.invokeMember("eval_expression", "run");
     var res = run.execute(10000);
     assertEquals("Array of the requested size computed", 10000, res.getArraySize());
 
     Predicate<SourceSection> isLambda = (ss) -> {
+      var sameSrc = ss.getSource().getCharacters().toString().equals(src.getCharacters().toString());
       var st = ss.getCharacters().toString();
-      return st.contains("noise") && !st.contains("map");
+      return sameSrc && st.contains("noise") && !st.contains("map");
     };
 
     assertAvoidIdInstrumentationTag(isLambda);
@@ -86,6 +81,7 @@ private void assertAvoidIdInstrumentationTag(Predicate<SourceSection> isLambda)
     var found = nodes.assertNewNodes("Give me nodes", 0, 10000);
     var err = new StringBuilder();
     var missingTagInLambda = false;
+    var count = 0;
     for (var nn : found.values()) {
       for (var n : nn) {
         var ss = n.getSourceSection();
@@ -98,6 +94,8 @@ private void assertAvoidIdInstrumentationTag(Predicate<SourceSection> isLambda)
             final boolean hasAvoidIdInstrumentationTag = in.hasTag(AvoidIdInstrumentationTag.class);
             if (!hasAvoidIdInstrumentationTag) {
               missingTagInLambda = true;
+            } else {
+              count++;
             }
 
             err.append("\n").append("  AvoidIdInstrumentationTag: ").append(hasAvoidIdInstrumentationTag);
@@ -115,5 +113,6 @@ private void assertAvoidIdInstrumentationTag(Predicate<SourceSection> isLambda)
     if (missingTagInLambda) {
       fail(err.toString());
     }
+    assertNotEquals("Found some nodes", 0, count);
   }
 }
diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/EnsoRootNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/EnsoRootNode.java
@@ -4,6 +4,7 @@
 import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.source.Source;
 import com.oracle.truffle.api.source.SourceSection;
+import java.util.Objects;
 import org.enso.interpreter.EnsoLanguage;
 import org.enso.interpreter.runtime.EnsoContext;
 import org.enso.interpreter.runtime.scope.LocalScope;
@@ -35,6 +36,7 @@ protected EnsoRootNode(
       String name,
       SourceSection sourceSection) {
     super(language, localScope.frameDescriptor());
+    Objects.requireNonNull(language);
     this.name = name;
     this.localScope = localScope;
     this.moduleScope = moduleScope;

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/ExpressionNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/ExpressionNode.java
@@ -191,7 +191,10 @@ public boolean hasTag(Class<? extends Tag> tag) {
     if (AvoidIdInstrumentationTag.class == tag) {
       return getRootNode() instanceof ClosureRootNode c && !c.isSubjectToInstrumentation();
     }
-    return tag == StandardTags.ExpressionTag.class || (tag == IdentifiedTag.class && id != null);
+    if (tag == StandardTags.ExpressionTag.class) {
+      return getSourceSection() != null;
+    }
+    return tag == IdentifiedTag.class && id != null;
   }
 
   /**

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java
@@ -7,6 +7,7 @@
 import com.oracle.truffle.api.instrumentation.Tag;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.NodeInfo;
+import com.oracle.truffle.api.source.SourceSection;
 import java.util.Set;
 import org.enso.interpreter.node.ExpressionNode;
 import org.enso.interpreter.runtime.EnsoContext;
@@ -65,14 +66,6 @@ public Object executeGeneric(VirtualFrame frame) {
   public InstrumentableNode materializeInstrumentableNodes(
       Set<Class<? extends Tag>> materializedTags) {
     if (materializedTags.contains(StandardTags.StatementTag.class)) {
-      var ctx = EnsoContext.get(this);
-      if (ctx != null) {
-        Assumption chromeInspectorNotAttached = ctx.getChromeInspectorNotAttached();
-        if (chromeInspectorNotAttached.isValid()
-            && ctx.getEnvironment().getInstruments().containsKey("inspect")) {
-          chromeInspectorNotAttached.invalidate("Chrome inspector attached");
-        }
-      }
       for (int i = 0; i < statements.length; i++) {
         if (!isNodeWrapped(statements[i])) {
           statements[i] = insert(StatementNode.wrap(statements[i]));
@@ -89,10 +82,20 @@ private static boolean isNodeWrapped(ExpressionNode node) {
     return node instanceof StatementNode || ExpressionNode.isWrapper(node);
   }
 
+  @Override
+  public SourceSection getSourceSection() {
+    var ss = super.getSourceSection();
+    return ss != null ? ss : getRootNode().getSourceSection();
+  }
+
   @Override
   public boolean hasTag(Class<? extends Tag> tag) {
-    return super.hasTag(tag)
-        || tag == StandardTags.RootBodyTag.class
-        || tag == StandardTags.RootTag.class;
+    if (super.hasTag(tag)) {
+      return true;
+    }
+    if (tag == StandardTags.RootBodyTag.class || tag == StandardTags.RootTag.class) {
+      return true;
+    }
+    return false;
   }
 }
diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/StatementNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/StatementNode.java
@@ -1,11 +1,14 @@
 package org.enso.interpreter.node.callable.function;
 
+import com.oracle.truffle.api.Assumption;
+import com.oracle.truffle.api.CompilerDirectives;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.instrumentation.StandardTags;
 import com.oracle.truffle.api.instrumentation.Tag;
 import com.oracle.truffle.api.source.SourceSection;
 import org.enso.interpreter.node.ClosureRootNode;
 import org.enso.interpreter.node.ExpressionNode;
+import org.enso.interpreter.runtime.EnsoContext;
 import org.enso.interpreter.runtime.tag.AvoidIdInstrumentationTag;
 
 /**
@@ -39,6 +42,14 @@ public boolean isInstrumentable() {
 
   @Override
   public Object executeGeneric(VirtualFrame frame) {
+    if (CompilerDirectives.inInterpreter()) {
+      var ctx = EnsoContext.get(this);
+      Assumption chromeInspectorNotAttached = ctx.getChromeInspectorNotAttached();
+      if (chromeInspectorNotAttached.isValid()
+          && ctx.getEnvironment().getInstruments().containsKey("inspect")) {
+        chromeInspectorNotAttached.invalidate("Chrome inspector attached");
+      }
+    }
     return node.executeGeneric(frame);
   }
 

diff --git a/...ne/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/BranchResult.java b/...ne/runtime/src/main/java/org/enso/interpreter/node/controlflow/caseexpr/BranchResult.java
@@ -1,15 +1,15 @@
 package org.enso.interpreter.node.controlflow.caseexpr;
 
+import com.oracle.truffle.api.interop.TruffleObject;
 import com.oracle.truffle.api.nodes.Node;
 import org.enso.interpreter.runtime.EnsoContext;
 
-public record BranchResult(boolean isMatched, Object result) {
-
-  public static BranchResult failure(Node node) {
+record BranchResult(boolean isMatched, Object result) implements TruffleObject {
+  static BranchResult failure(Node node) {
     return new BranchResult(false, EnsoContext.get(node).getBuiltins().nothing());
   }
 
-  public static BranchResult success(Object result) {
+  static BranchResult success(Object result) {
     return new BranchResult(true, result);
   }
 

diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Builtin.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Builtin.java
@@ -19,10 +19,9 @@ public Cons(String name, String... params) {
       this(name, Arrays.asList(params));
     }
 
-    private AtomConstructor build(ModuleScope scope, Type type) {
+    private AtomConstructor build(EnsoLanguage language, ModuleScope scope, Type type) {
       var res = new AtomConstructor(name, scope, type,true);
-      res.initializeFields(
-          IntStream.range(0, params.size())
+      res.initializeFields(language, IntStream.range(0, params.size())
               .mapToObj(
                   i ->
                       new ArgumentDefinition(
@@ -66,7 +65,7 @@ public final void initialize(EnsoLanguage language, ModuleScope scope, Map<Class
       var conses = getDeclaredConstructors();
       constructors = new AtomConstructor[conses.size()];
       for (int i = 0; i < constructors.length; i++) {
-        var cons = conses.get(i).build(scope, type);
+        var cons = conses.get(i).build(language, scope, type);
         constructors[i] = cons;
         type.registerConstructor(cons);
       }