From 9a3980e0472ae138a57c4f358fdf91b7754db97d Mon Sep 17 00:00:00 2001 From: Flo Drux Date: Tue, 24 Sep 2024 07:54:17 +0200 Subject: [PATCH] cache target type --- .../types3/util/MapBasedTypeCheck3.java | 83 ++++++++++++++++--- .../types3/MapBasedTypeCheck3Test.java | 66 +++++++++++++++ 2 files changed, 139 insertions(+), 10 deletions(-) create mode 100644 monticore-grammar/src/test/java/de/monticore/types3/MapBasedTypeCheck3Test.java diff --git a/monticore-grammar/src/main/java/de/monticore/types3/util/MapBasedTypeCheck3.java b/monticore-grammar/src/main/java/de/monticore/types3/util/MapBasedTypeCheck3.java index a1f0872e1a..fefe59bfe1 100644 --- a/monticore-grammar/src/main/java/de/monticore/types3/util/MapBasedTypeCheck3.java +++ b/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 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); } diff --git a/monticore-grammar/src/test/java/de/monticore/types3/MapBasedTypeCheck3Test.java b/monticore-grammar/src/test/java/de/monticore/types3/MapBasedTypeCheck3Test.java new file mode 100644 index 0000000000..31a2d0ac4c --- /dev/null +++ b/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"); + } + +}