Merge branch 'cache-target-type' into 'dev'

cache target type

See merge request monticore/monticore!1031

monticore-grammar/src/main/java/de/monticore/types3/util/MapBasedTypeCheck3.java

@@ -14,6 +14,8 @@
 import de.monticore.visitor.ITraverser;
 import de.se_rwth.commons.logging.Log;
 
+import java.util.Optional;
+
 /**
  * A default implementation of the TypeCheck3 interface.
  * This class is designed to be derived from,
@@ -31,6 +33,8 @@
  */
 public class MapBasedTypeCheck3 extends TypeCheck3 {
 
+  protected static final String LOG_NAME = "MapBasedTypeCheck3";
+
   protected ITraverser typeTraverser;
 
   protected Type4Ast type4Ast;
@@ -106,13 +110,21 @@ public SymTypeExpression _symTypeFromAST(ASTMCQualifiedName mcQualifiedName) {
 
   @Override
   public SymTypeExpression _typeOf(ASTExpression expr) {
-    // reset, if target typing had been used, as currently,
+    // do not reset if target typing had been used, as currently,
     // there is no target type
-    if (getCtx4Ast().getContextOfExpression(expr).hasTargetType()) {
-      getType4Ast().reset(expr);
-      getCtx4Ast().reset(expr);
+    if (getCtx4Ast().getContextOfExpression(expr).hasTargetType() &&
+        getType4Ast().hasPartialTypeOfExpression(expr)) {
+      Log.trace(
+          expr.get_SourcePositionStart() + "-"
+              + expr.get_SourcePositionEnd() + " typeof(): "
+              + "no target type given, but had been given ("
+              + getCtx4Ast().getContextOfExpression(expr)
+              .getTargetType().printFullName()
+              + "), using the cached result;",
+          LOG_NAME
+      );
     }
-    if (!getType4Ast().hasTypeOfExpression(expr)) {
+    if (!getType4Ast().hasPartialTypeOfExpression(expr)) {
       expr.accept(getTypeTraverser());
     }
     return getType4Ast().getPartialTypeOfExpr(expr);
@@ -123,11 +135,62 @@ public SymTypeExpression _typeOf(
       ASTExpression expr,
       SymTypeExpression targetType
   ) {
-    // need to reset in case the target type changes the typing in the AST
-    getType4Ast().reset(expr);
-    getCtx4Ast().reset(expr);
-    getCtx4Ast().setTargetTypeOfExpression(expr, targetType);
-    expr.accept(getTypeTraverser());
+    // if some other (or no) target type had been used before,
+    // then something questionable is happening (this is not expected)
+    Optional<SymTypeExpression> targetTypeOld =
+        getCtx4Ast().getContextOfExpression(expr).hasTargetType() ?
+            Optional.of(getCtx4Ast().getContextOfExpression(expr).getTargetType()) :
+            Optional.empty();
+    if (getType4Ast().hasPartialTypeOfExpression(expr) && targetTypeOld.isEmpty()) {
+      Log.error("0xFD111 internal error: "
+              + "called typeof() with target type " + targetType.printFullName()
+              + ", however, typeof() for the same expression"
+              + " had already been called without target type"
+              + ", which is not expected;"
+              + " A call to typeof() with the target type must be "
+              + " before every call to typeof without target type"
+              + ", such that the corresponding result can be cached."
+              + System.lineSeparator()
+              + "  1. Check the order of calls to typeof() (e.g., in CoCos)."
+              + System.lineSeparator()
+              + "  2. It is recommended to have a separate visitor"
+              + " to call typeof() with target type before running"
+              + " any other visitors relying on typeof()"
+              + ", such that they can use the cached value.",
+          expr.get_SourcePositionStart(),
+          expr.get_SourcePositionEnd()
+      );
+      // need to reset in case the target type changes the typing in the AST
+      getType4Ast().reset(expr);
+      getCtx4Ast().reset(expr);
+    }
+    else if (getType4Ast().hasPartialTypeOfExpression(expr) &&
+        targetTypeOld.isPresent() &&
+        !targetTypeOld.get().deepEquals(targetType)
+    ) {
+      Log.error("0xFD112 internal error: "
+              + "called typeof() with target type " + targetType.printFullName()
+              + ", however, typeof() for the same expression"
+              + " had already been called with target type "
+              + targetTypeOld.get().printFullName()
+              + ", which is not expected;"
+              + " Every expression must have at most one target type,"
+              + " as the typeof()-results are cached."
+              + " For special cases such as modeling languages with variability, "
+              + "it is suggested to either reset the maps"
+              + " used in this TypeCheck implementation"
+              + " or reset the TypeCheck altogether between runs.",
+          expr.get_SourcePositionStart(),
+          expr.get_SourcePositionEnd()
+      );
+      // need to reset in case the target type changes the typing in the AST
+      getType4Ast().reset(expr);
+      getCtx4Ast().reset(expr);
+    }
+    if (!getType4Ast().hasPartialTypeOfExpression(expr)) {
+      getCtx4Ast().setTargetTypeOfExpression(expr, targetType);
+      expr.accept(getTypeTraverser());
+    }
     return getType4Ast().getPartialTypeOfExpr(expr);
   }
 

monticore-grammar/src/test/java/de/monticore/types3/MapBasedTypeCheck3Test.java

@@ -0,0 +1,66 @@
+/* (c) https://github.com/MontiCore/monticore */
+package de.monticore.types3;
+
+import de.monticore.expressions.expressionsbasis._ast.ASTExpression;
+import de.monticore.types.check.SymTypeExpression;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+
+import static de.monticore.types.check.SymTypeExpressionFactory.createGenerics;
+import static de.monticore.types3.util.DefsTypesForTests._floatSymType;
+import static de.monticore.types3.util.DefsTypesForTests._intSymType;
+import static de.monticore.types3.util.DefsTypesForTests._unboxedListSymType;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class MapBasedTypeCheck3Test extends AbstractTypeVisitorTest {
+
+  // tests calling the same expression with typeof multiple times,
+  // as this may happen due to CoCos
+
+  @Test
+  public void typeOfMultipleCallsNoTargetType() throws IOException {
+    ASTExpression expr = parseExpr("8243721");
+    TypeCheck3.typeOf(expr);
+    TypeCheck3.typeOf(expr);
+    assertNoFindings();
+  }
+
+  @Test
+  public void typeOfMultipleCallsTargetTypeFirst() throws IOException {
+    ASTExpression expr = parseExpr("[]");
+    SymTypeExpression intList =
+        createGenerics(_unboxedListSymType.getTypeInfo(), _intSymType);
+    SymTypeExpression firstRes = TypeCheck3.typeOf(expr, intList);
+    // Cached value gets reused here.
+    // Thus, the target type is only required once.
+    SymTypeExpression secondRes = TypeCheck3.typeOf(expr);
+    assertTrue(secondRes.deepEquals(firstRes));
+    assertNoFindings();
+  }
+
+  @Test
+  public void typeOfMultipleCallsSameTargetType() throws IOException {
+    ASTExpression expr = parseExpr("313");
+    TypeCheck3.typeOf(expr, _intSymType);
+    TypeCheck3.typeOf(expr, _intSymType);
+    assertNoFindings();
+  }
+
+  @Test
+  public void typeOfMultipleCallsTargetTypeSecond() throws IOException {
+    ASTExpression expr = parseExpr("42");
+    TypeCheck3.typeOf(expr);
+    TypeCheck3.typeOf(expr, _intSymType);
+    assertHasErrorCode("0xFD111");
+  }
+
+  @Test
+  public void typeOfMultipleCallsDifferentTargetType() throws IOException {
+    ASTExpression expr = parseExpr("4");
+    TypeCheck3.typeOf(expr, _intSymType);
+    TypeCheck3.typeOf(expr, _floatSymType);
+    assertHasErrorCode("0xFD112");
+  }
+
+}