From ea8a6cc5989f2371930b3e63b7847d745aac2bf5 Mon Sep 17 00:00:00 2001 From: Marc Auberer Date: Sat, 11 Jan 2025 23:56:21 +0100 Subject: [PATCH] Fix const variable reassign reporting bug (#703) --- src/symboltablebuilder/SymbolTableEntry.cpp | 3 - src/typechecker/OpRuleManager.cpp | 16 ++-- src/typechecker/OpRuleManager.h | 14 ++-- src/typechecker/TypeChecker.cpp | 70 ++++++++--------- .../exception.out | 3 + .../error-reassign-const-var/exception.out | 77 ++++++++++++++++++- .../error-reassign-const-var/source.spice | 12 +++ 7 files changed, 140 insertions(+), 55 deletions(-) diff --git a/src/symboltablebuilder/SymbolTableEntry.cpp b/src/symboltablebuilder/SymbolTableEntry.cpp index 55f9c1f7e..c2a7b614d 100644 --- a/src/symboltablebuilder/SymbolTableEntry.cpp +++ b/src/symboltablebuilder/SymbolTableEntry.cpp @@ -38,9 +38,6 @@ void SymbolTableEntry::updateType(const QualType &newType, [[maybe_unused]] bool */ void SymbolTableEntry::updateState(const LifecycleState &newState, const ASTNode *node, bool force) { const LifecycleState oldState = lifecycle.getCurrentState(); - // Check if this is a constant variable and is already initialized - if (newState != DEAD && oldState != DECLARED && qualType.isConst() && !force) // GCOV_EXCL_LINE - throw CompilerError(INTERNAL_ERROR, "Not re-assignable variable '" + name + "'"); // GCOV_EXCL_LINE if (newState == DEAD && oldState == DECLARED) // GCOV_EXCL_LINE throw CompilerError(INTERNAL_ERROR, "Cannot destroy uninitialized variable '" + name + "'"); // GCOV_EXCL_LINE if (newState == DEAD && oldState == DEAD) // GCOV_EXCL_LINE diff --git a/src/typechecker/OpRuleManager.cpp b/src/typechecker/OpRuleManager.cpp index 0656672aa..b44af2000 100644 --- a/src/typechecker/OpRuleManager.cpp +++ b/src/typechecker/OpRuleManager.cpp @@ -232,7 +232,7 @@ ExprResult OpRuleManager::getDivEqualResultType(ASTNode *node, const ExprResult return {validateBinaryOperation(node, DIV_EQUAL_OP_RULES, std::size(DIV_EQUAL_OP_RULES), "/=", lhsType, rhsType)}; } -QualType OpRuleManager::getRemEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getRemEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -243,7 +243,7 @@ QualType OpRuleManager::getRemEqualResultType(const ASTNode *node, const ExprRes return validateBinaryOperation(node, REM_EQUAL_OP_RULES, std::size(REM_EQUAL_OP_RULES), "%=", lhsType, rhsType); } -QualType OpRuleManager::getSHLEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getSHLEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -254,7 +254,7 @@ QualType OpRuleManager::getSHLEqualResultType(const ASTNode *node, const ExprRes return validateBinaryOperation(node, SHL_EQUAL_OP_RULES, std::size(SHL_EQUAL_OP_RULES), "<<=", lhsType, rhsType); } -QualType OpRuleManager::getSHREqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getSHREqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -265,7 +265,7 @@ QualType OpRuleManager::getSHREqualResultType(const ASTNode *node, const ExprRes return validateBinaryOperation(node, SHR_EQUAL_OP_RULES, std::size(SHR_EQUAL_OP_RULES), ">>=", lhsType, rhsType); } -QualType OpRuleManager::getAndEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getAndEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -276,7 +276,7 @@ QualType OpRuleManager::getAndEqualResultType(const ASTNode *node, const ExprRes return validateBinaryOperation(node, AND_EQUAL_OP_RULES, std::size(AND_EQUAL_OP_RULES), "&=", lhsType, rhsType); } -QualType OpRuleManager::getOrEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getOrEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -287,7 +287,7 @@ QualType OpRuleManager::getOrEqualResultType(const ASTNode *node, const ExprResu return validateBinaryOperation(node, OR_EQUAL_OP_RULES, std::size(OR_EQUAL_OP_RULES), "|=", lhsType, rhsType); } -QualType OpRuleManager::getXorEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) { +QualType OpRuleManager::getXorEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const { // Check if we try to assign a constant value ensureNoConstAssign(node, lhs.type); @@ -787,11 +787,11 @@ void OpRuleManager::ensureUnsafeAllowed(const ASTNode *node, const char *name, c SOFT_ERROR_VOID(node, UNSAFE_OPERATION_IN_SAFE_CONTEXT, errorMsg) } -void OpRuleManager::ensureNoConstAssign(const ASTNode *node, const QualType &lhs, bool isDecl, bool isReturn) { +void OpRuleManager::ensureNoConstAssign(const ASTNode *node, const QualType &lhs, bool isDecl, bool isReturn) const { // Check if we try to assign a constant value if (lhs.removeReferenceWrapper().isConst() && !isDecl && !isReturn) { const std::string errorMessage = "Trying to assign value to an immutable variable of type " + lhs.getName(true); - throw SemanticError(node, REASSIGN_CONST_VARIABLE, errorMessage); + SOFT_ERROR_VOID(node, REASSIGN_CONST_VARIABLE, errorMessage); } } diff --git a/src/typechecker/OpRuleManager.h b/src/typechecker/OpRuleManager.h index e104eb49b..9d282843c 100644 --- a/src/typechecker/OpRuleManager.h +++ b/src/typechecker/OpRuleManager.h @@ -602,12 +602,12 @@ class OpRuleManager { ExprResult getMinusEqualResultType(ASTNode *node, const ExprResult &lhs, const ExprResult &rhs, size_t opIdx); ExprResult getMulEqualResultType(ASTNode *node, const ExprResult &lhs, const ExprResult &rhs, size_t opIdx); ExprResult getDivEqualResultType(ASTNode *node, const ExprResult &lhs, const ExprResult &rhs, size_t opIdx); - static QualType getRemEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); - static QualType getSHLEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); - static QualType getSHREqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); - static QualType getAndEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); - static QualType getOrEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); - static QualType getXorEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); + QualType getRemEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; + QualType getSHLEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; + QualType getSHREqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; + QualType getAndEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; + QualType getOrEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; + QualType getXorEqualResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs) const; static QualType getLogicalOrResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); static QualType getLogicalAndResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); static QualType getBitwiseOrResultType(const ASTNode *node, const ExprResult &lhs, const ExprResult &rhs); @@ -658,7 +658,7 @@ class OpRuleManager { const char *messagePrefix); void ensureUnsafeAllowed(const ASTNode *node, const char *name, const QualType &lhs) const; void ensureUnsafeAllowed(const ASTNode *node, const char *name, const QualType &lhs, const QualType &rhs) const; - static void ensureNoConstAssign(const ASTNode *node, const QualType &lhs, bool isDecl = false, bool isReturn = false); + void ensureNoConstAssign(const ASTNode *node, const QualType &lhs, bool isDecl = false, bool isReturn = false) const; }; } // namespace spice::compiler \ No newline at end of file diff --git a/src/typechecker/TypeChecker.cpp b/src/typechecker/TypeChecker.cpp index 1b4c9e255..e9288829a 100644 --- a/src/typechecker/TypeChecker.cpp +++ b/src/typechecker/TypeChecker.cpp @@ -937,17 +937,17 @@ std::any TypeChecker::visitAssignExpr(AssignExprNode *node) { } else if (node->op == AssignExprNode::OP_DIV_EQUAL) { rhsType = opRuleManager.getDivEqualResultType(node, lhs, rhs, 0).type; } else if (node->op == AssignExprNode::OP_REM_EQUAL) { - rhsType = OpRuleManager::getRemEqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getRemEqualResultType(node, lhs, rhs); } else if (node->op == AssignExprNode::OP_SHL_EQUAL) { - rhsType = OpRuleManager::getSHLEqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getSHLEqualResultType(node, lhs, rhs); } else if (node->op == AssignExprNode::OP_SHR_EQUAL) { - rhsType = OpRuleManager::getSHREqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getSHREqualResultType(node, lhs, rhs); } else if (node->op == AssignExprNode::OP_AND_EQUAL) { - rhsType = OpRuleManager::getAndEqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getAndEqualResultType(node, lhs, rhs); } else if (node->op == AssignExprNode::OP_OR_EQUAL) { - rhsType = OpRuleManager::getOrEqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getOrEqualResultType(node, lhs, rhs); } else if (node->op == AssignExprNode::OP_XOR_EQUAL) { - rhsType = OpRuleManager::getXorEqualResultType(node, lhs, rhs); + rhsType = opRuleManager.getXorEqualResultType(node, lhs, rhs); } if (lhsVar) { // Variable is involved on the left side @@ -1335,18 +1335,18 @@ std::any TypeChecker::visitPostfixUnaryExpr(PostfixUnaryExprNode *node) { // Visit left side PostfixUnaryExprNode *lhsNode = node->postfixUnaryExpr; - auto lhs = std::any_cast(visit(lhsNode)); - auto [lhsType, lhsEntry] = lhs; - HANDLE_UNRESOLVED_TYPE_ER(lhsType) + auto operand = std::any_cast(visit(lhsNode)); + auto [operandType, operandEntry] = operand; + HANDLE_UNRESOLVED_TYPE_ER(operandType) switch (node->op) { case PostfixUnaryExprNode::OP_SUBSCRIPT: { - lhsType = lhsType.removeReferenceWrapper(); + operandType = operandType.removeReferenceWrapper(); // Check if we can apply the subscript operator on the lhs type - if (!lhsType.isOneOf({TY_ARRAY, TY_STRING, TY_PTR})) + if (!operandType.isOneOf({TY_ARRAY, TY_STRING, TY_PTR})) SOFT_ERROR_ER(node, OPERATOR_WRONG_DATA_TYPE, - "Can only apply subscript operator on array type, got " + lhsType.getName(true)) + "Can only apply subscript operator on array type, got " + operandType.getName(true)) // Visit index assignment AssignExprNode *indexAssignExpr = node->subscriptIndexExpr; @@ -1357,15 +1357,15 @@ std::any TypeChecker::visitPostfixUnaryExpr(PostfixUnaryExprNode *node) { SOFT_ERROR_ER(node, ARRAY_INDEX_NOT_INT_OR_LONG, "Array index must be of type int or long") // Check if we have an unsafe operation - if (lhsType.isPtr() && !currentScope->doesAllowUnsafeOperations()) + if (operandType.isPtr() && !currentScope->doesAllowUnsafeOperations()) SOFT_ERROR_ER( node, UNSAFE_OPERATION_IN_SAFE_CONTEXT, "The subscript operator on pointers is an unsafe operation. Use unsafe blocks if you know what you are doing.") // Check if we have a hardcoded array index - if (lhsType.isArray() && lhsType.getArraySize() != ARRAY_SIZE_UNKNOWN && indexAssignExpr->hasCompileTimeValue()) { + if (operandType.isArray() && operandType.getArraySize() != ARRAY_SIZE_UNKNOWN && indexAssignExpr->hasCompileTimeValue()) { const int32_t constIndex = indexAssignExpr->getCompileTimeValue().intValue; - const unsigned int constSize = lhsType.getArraySize(); + const unsigned int constSize = operandType.getArraySize(); // Check if we are accessing out-of-bounds memory if (constIndex >= static_cast(constSize)) { const std::string idxStr = std::to_string(constIndex); @@ -1376,10 +1376,10 @@ std::any TypeChecker::visitPostfixUnaryExpr(PostfixUnaryExprNode *node) { } // Get item type - lhsType = lhsType.getContained(); + operandType = operandType.getContained(); // Remove heap specifier - lhsType.getSpecifiers().isHeap = false; + operandType.getSpecifiers().isHeap = false; break; } @@ -1387,10 +1387,10 @@ std::any TypeChecker::visitPostfixUnaryExpr(PostfixUnaryExprNode *node) { const std::string &fieldName = node->identifier; // Check if lhs is enum or strobj - QualType lhsBaseTy = lhsType; + QualType lhsBaseTy = operandType; autoDeReference(lhsBaseTy); if (!lhsBaseTy.is(TY_STRUCT)) - SOFT_ERROR_ER(node, INVALID_MEMBER_ACCESS, "Cannot apply member access operator on " + lhsType.getName(false)) + SOFT_ERROR_ER(node, INVALID_MEMBER_ACCESS, "Cannot apply member access operator on " + operandType.getName(false)) // Retrieve registry entry const std::string &structName = lhsBaseTy.getSubType(); @@ -1420,50 +1420,48 @@ std::any TypeChecker::visitPostfixUnaryExpr(PostfixUnaryExprNode *node) { memberEntry->used = true; // Overwrite type and entry of left side with member type and entry - lhsType = memberType; - lhsEntry = memberEntry; + operandType = memberType; + operandEntry = memberEntry; break; } case PostfixUnaryExprNode::OP_PLUS_PLUS: { - if (lhsEntry) { + operandType = opRuleManager.getPostfixPlusPlusResultType(node, operand, 0).type; + + if (operandEntry) { // In case the lhs is captured, notify the capture about the write access - if (Capture *lhsCapture = currentScope->symbolTable.lookupCapture(lhsEntry->name); lhsCapture) + if (Capture *lhsCapture = currentScope->symbolTable.lookupCapture(operandEntry->name); lhsCapture) lhsCapture->setAccessType(READ_WRITE); // Update the state of the variable - lhsEntry->updateState(INITIALIZED, node, false); + operandEntry->updateState(INITIALIZED, node, false); } - auto [type, entry] = opRuleManager.getPostfixPlusPlusResultType(node, lhs, 0); - lhsType = type; - lhsEntry = entry; break; } case PostfixUnaryExprNode::OP_MINUS_MINUS: { - if (lhsEntry) { + operandType = opRuleManager.getPostfixMinusMinusResultType(node, operand, 0).type; + + if (operandEntry) { // In case the lhs is captured, notify the capture about the write access - if (Capture *lhsCapture = currentScope->symbolTable.lookupCapture(lhsEntry->name); lhsCapture) + if (Capture *lhsCapture = currentScope->symbolTable.lookupCapture(operandEntry->name); lhsCapture) lhsCapture->setAccessType(READ_WRITE); // Update the state of the variable - lhsEntry->updateState(INITIALIZED, node, false); + operandEntry->updateState(INITIALIZED, node, false); } - ExprResult result = opRuleManager.getPostfixMinusMinusResultType(node, lhs, 0); - lhsType = result.type; - lhsEntry = result.entry; break; } default: throw CompilerError(UNHANDLED_BRANCH, "PostfixUnaryExpr fall-through"); // GCOV_EXCL_LINE } - if (lhsType.is(TY_INVALID)) { - const std::string varName = lhsEntry ? lhsEntry->name : ""; + if (operandType.is(TY_INVALID)) { + const std::string varName = operandEntry ? operandEntry->name : ""; SOFT_ERROR_ER(node, REFERENCED_UNDEFINED_VARIABLE, "Variable '" + varName + "' was referenced before declared") } - return ExprResult{node->setEvaluatedSymbolType(lhsType, manIdx), lhsEntry}; + return ExprResult{node->setEvaluatedSymbolType(operandType, manIdx), operandEntry}; } std::any TypeChecker::visitAtomicExpr(AtomicExprNode *node) { diff --git a/test/test-files/typechecker/variables/error-reassign-const-ref-var/exception.out b/test/test-files/typechecker/variables/error-reassign-const-ref-var/exception.out index e07fbccd8..14c58e3b6 100644 --- a/test/test-files/typechecker/variables/error-reassign-const-ref-var/exception.out +++ b/test/test-files/typechecker/variables/error-reassign-const-ref-var/exception.out @@ -1,3 +1,6 @@ +[Error|Compiler]: +Unresolved soft errors: There are unresolved errors. Please fix them and recompile. + [Error|Semantic] ./source.spice:2:5: Cannot re-assign constant variable: Trying to assign value to an immutable variable of type const bool& diff --git a/test/test-files/typechecker/variables/error-reassign-const-var/exception.out b/test/test-files/typechecker/variables/error-reassign-const-var/exception.out index 5338d5f04..b8c6eaa15 100644 --- a/test/test-files/typechecker/variables/error-reassign-const-var/exception.out +++ b/test/test-files/typechecker/variables/error-reassign-const-var/exception.out @@ -1,5 +1,80 @@ +[Error|Compiler]: +Unresolved soft errors: There are unresolved errors. Please fix them and recompile. + [Error|Semantic] ./source.spice:3:5: Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int 3 i = 1234; - ^^^^^^^^ \ No newline at end of file + ^^^^^^^^ + +[Error|Semantic] ./source.spice:4:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +4 i++; + ^^^ + +[Error|Semantic] ./source.spice:5:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +5 i--; + ^^^ + +[Error|Semantic] ./source.spice:6:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +6 ++i; + ^^^ + +[Error|Semantic] ./source.spice:7:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +7 --i; + ^^^ + +[Error|Semantic] ./source.spice:8:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +8 i += 2; + ^^^^^^ + +[Error|Semantic] ./source.spice:9:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +9 i -= 2; + ^^^^^^ + +[Error|Semantic] ./source.spice:10:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +10 i *= 2; + ^^^^^^ + +[Error|Semantic] ./source.spice:11:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +11 i /= 2; + ^^^^^^ + +[Error|Semantic] ./source.spice:12:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +12 i %= 2; + ^^^^^^ + +[Error|Semantic] ./source.spice:13:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +13 i ^= 3; + ^^^^^^ + +[Error|Semantic] ./source.spice:14:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +14 i <<= 2; + ^^^^^^^ + +[Error|Semantic] ./source.spice:15:5: +Cannot re-assign constant variable: Trying to assign value to an immutable variable of type int + +15 i >>= 2; + ^^^^^^^ \ No newline at end of file diff --git a/test/test-files/typechecker/variables/error-reassign-const-var/source.spice b/test/test-files/typechecker/variables/error-reassign-const-var/source.spice index d703c749c..002c7fe3d 100644 --- a/test/test-files/typechecker/variables/error-reassign-const-var/source.spice +++ b/test/test-files/typechecker/variables/error-reassign-const-var/source.spice @@ -1,4 +1,16 @@ f main() { const int i = 123; i = 1234; + i++; + i--; + ++i; + --i; + i += 2; + i -= 2; + i *= 2; + i /= 2; + i %= 2; + i ^= 3; + i <<= 2; + i >>= 2; } \ No newline at end of file