Fix const variable reassign reporting bug (#703)

spicelang · Jan 11, 2025 · ea8a6cc · ea8a6cc
1 parent 70ef9af
commit ea8a6cc
Show file tree

Hide file tree

Showing 7 changed files with 140 additions and 55 deletions.
diff --git 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
@@ -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
@@ -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
diff --git 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<ExprResult>(visit(lhsNode));
-  auto [lhsType, lhsEntry] = lhs;
-  HANDLE_UNRESOLVED_TYPE_ER(lhsType)
+  auto operand = std::any_cast<ExprResult>(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<int32_t>(constSize)) {
         const std::string idxStr = std::to_string(constIndex);
@@ -1376,21 +1376,21 @@ 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;
   }
   case PostfixUnaryExprNode::OP_MEMBER_ACCESS: {
     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
@@ -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
@@ -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;
-   ^^^^^^^^
+   ^^^^^^^^
+
+[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;
+    ^^^^^^^
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
@@ -1,4 +1,16 @@
 f<int> 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;
 }