Fix bug where the ctor for structs with VTable was not enforced (#520)

spicelang · Apr 21, 2024 · 230e6e7 · 230e6e7
1 parent 9c9cf28
commit 230e6e7
Show file tree

Hide file tree

Showing 10 changed files with 118 additions and 27 deletions.
diff --git a/media/test-project/test.spice b/media/test-project/test.spice
@@ -1,18 +1,38 @@
+type Visitor struct {}
+
+type SymbolTable struct {}
+
+type Visitable interface {
+    f<bool> accept(Visitor*);
+}
+
+type AstNode struct : Visitable {}
+
+type AstEntryNode struct : Visitable {
+    AstNode astNode
+    SymbolTable* extFunctionScope
+    bool takesArgs
+}
+
 f<int> main() {
-    int[3] a = [1, 2, 3];
-    int* aPtr = a;
-    printf("%d\n", *aPtr);
-    unsafe {
-        aPtr++;
-    }
-    printf("%d\n", *aPtr);
+    AstEntryNode entryNode;
+    printf("%d", entryNode.takesArgs);
 }
 
 /*import "bootstrap/util/block-allocator";
 import "bootstrap/util/memory";
 
+type ASTNode struct {
+    int value
+}
+
+p ASTNode.dtor() {
+    printf("Dtor called!");
+}
+
 f<int> main() {
     DefaultMemoryManager defaultMemoryManager;
     IMemoryManager* memoryManager = &defaultMemoryManager;
-    BlockAllocator<int> allocator = BlockAllocator<int>(memoryManager, 10l);
+    memoryManager.allocate(10l);
+    //BlockAllocator<ASTNode> allocator = BlockAllocator<ASTNode>(memoryManager, 10l);
 }*/
diff --git a/src-bootstrap/util/block-allocator.spice b/src-bootstrap/util/block-allocator.spice
@@ -1,6 +1,5 @@
 // Std imports
 import "std/data/vector";
-import "std/type/long";
 import "std/os/system";
 
 // Own imports
@@ -10,7 +9,7 @@ type Base dyn;
 
 public type BlockAllocator<Base> struct {
     IMemoryManager* memoryManager
-    Vector<heap byte*> memoryBlocks
+    Vector<byte*> memoryBlocks
     Vector<Base*> allocatedObjects
     unsigned long blockSize
     unsigned long offsetInBlock = 0l

diff --git a/src/symboltablebuilder/Scope.cpp b/src/symboltablebuilder/Scope.cpp
@@ -324,6 +324,25 @@ std::vector<Function *> Scope::getVirtualMethods() {
   return methods;
 }
 
+/**
+ * Retrieve all struct manifestations in this scope in the order of their declaration
+ *
+ * @return All struct manifestations in declaration order
+ */
+std::vector<const Struct *> Scope::getAllStructManifestationsInDeclarationOrder() const {
+  // Retrieve all struct manifestations in this scope
+  std::vector<const Struct *> manifestations;
+  manifestations.reserve(structs.size()); // Reserve at least the size of individual generic structs
+  for (const auto &[structId, structManifestations] : structs)
+    for (const auto &[_, manifestation] : structManifestations)
+      manifestations.push_back(&manifestation);
+
+  // Sort manifestations by declaration code location
+  auto sortLambda = [](const Struct *lhs, const Struct *rhs) { return lhs->getDeclCodeLoc() < rhs->getDeclCodeLoc(); };
+  std::ranges::sort(manifestations, sortLambda);
+  return manifestations;
+}
+
 /**
  * Check if this struct has any reference fields
  *

diff --git a/src/symboltablebuilder/Scope.h b/src/symboltablebuilder/Scope.h
@@ -80,6 +80,7 @@ class Scope {
   void ensureSuccessfulTypeInference() const;
   [[nodiscard]] size_t getFieldCount() const;
   [[nodiscard]] std::vector<Function *> getVirtualMethods();
+  [[nodiscard]] std::vector<const Struct *> getAllStructManifestationsInDeclarationOrder() const;
   [[nodiscard]] bool hasRefFields();
   [[nodiscard]] unsigned int getLoopNestingDepth() const;
   [[nodiscard]] bool isInCaseBranch() const;

diff --git a/src/typechecker/TypeChecker.cpp b/src/typechecker/TypeChecker.cpp
@@ -30,12 +30,11 @@ std::any TypeChecker::visitEntry(EntryNode *node) {
 
   // Check which implicit structures we need for each struct, defined in this source file
   if (isPrepare) {
-    for (const auto &[structName, manifestations] : rootScope->getStructs()) {
-      for (const auto &[_, manifestation] : manifestations) {
-        createDefaultCtorIfRequired(manifestation, manifestation.scope);
-        createDefaultCopyCtorIfRequired(manifestation, manifestation.scope);
-        createDefaultDtorIfRequired(manifestation, manifestation.scope);
-      }
+    for (const Struct *manifestation : rootScope->getAllStructManifestationsInDeclarationOrder()) {
+      // Check if we need to create a default ctor, copy ctor or dtor
+      createDefaultCtorIfRequired(*manifestation, manifestation->scope);
+      createDefaultCopyCtorIfRequired(*manifestation, manifestation->scope);
+      createDefaultDtorIfRequired(*manifestation, manifestation->scope);
     }
   }
 
@@ -596,8 +595,9 @@ std::any TypeChecker::visitDeclStmt(DeclStmtNode *node) {
     if (localVarType.is(TY_STRUCT) && !node->isParam && !node->isForEachItem) {
       Scope *matchScope = localVarType.getBodyScope();
       assert(matchScope != nullptr);
-      // Check if we need to call a ctor
-      node->isCtorCallRequired = matchScope->hasRefFields();
+      // Check if we are required to call a ctor
+      auto structDeclNode = spice_pointer_cast<StructDefNode *>(localVarType.getStruct(node)->declNode);
+      node->isCtorCallRequired = matchScope->hasRefFields() || structDeclNode->emitVTable;
       // Check if we have a no-args ctor to call
       const std::string &structName = localVarType.getSubType();
       const SymbolType &thisType = localVarType;

diff --git a/src/typechecker/TypeCheckerImplicit.cpp b/src/typechecker/TypeCheckerImplicit.cpp
@@ -64,7 +64,7 @@ void TypeChecker::createDefaultStructMethod(const Struct &spiceStruct, const std
  * @param structScope Scope of the struct
  */
 void TypeChecker::createDefaultCtorIfRequired(const Struct &spiceStruct, Scope *structScope) {
-  ASTNode *node = spiceStruct.declNode;
+  auto node = spice_pointer_cast<StructDefNode *>(spiceStruct.declNode);
   assert(structScope != nullptr && structScope->type == ScopeType::STRUCT);
 
   // Abort if the struct already has a user-defined constructor
@@ -93,16 +93,23 @@ void TypeChecker::createDefaultCtorIfRequired(const Struct &spiceStruct, Scope *
     }
 
     if (fieldSymbol->getType().is(TY_STRUCT)) {
-      Scope *fieldScope = fieldSymbol->getType().getBodyScope();
+      Scope *bodyScope = fieldSymbol->getType().getBodyScope();
+      Struct *fieldStruct = fieldSymbol->getType().getStruct(node);
+      // Check if we are required to call a ctor
+      const auto structDeclNode = spice_pointer_cast<StructDefNode *>(fieldStruct->declNode);
+      const bool isCtorCallRequired = bodyScope->hasRefFields() || structDeclNode->emitVTable;
       // Lookup ctor function
-      const Function *ctorFct = FunctionManager::matchFunction(fieldScope, CTOR_FUNCTION_NAME, thisType, {}, {}, true, node);
+      const Function *ctorFct = FunctionManager::matchFunction(bodyScope, CTOR_FUNCTION_NAME, thisType, {}, {}, true, node);
+      // If we are required to construct, but no constructor is found, we can't generate a default ctor for the outer struct
+      if (!ctorFct && isCtorCallRequired)
+        return;
       hasFieldsToConstruct |= ctorFct != nullptr;
       requestRevisitIfRequired(ctorFct);
     }
   }
 
   // If we don't have any fields, that require us to do anything in the ctor, we can skip it
-  if (!hasFieldsWithDefaultValue && !hasFieldsToConstruct)
+  if (!hasFieldsWithDefaultValue && !hasFieldsToConstruct && !node->emitVTable)
     return;
 
   // Create the default ctor function
@@ -118,7 +125,7 @@ void TypeChecker::createDefaultCtorBody(const Function *ctorFunction) { createCt
  * @param structScope Scope of the struct
  */
 void TypeChecker::createDefaultCopyCtorIfRequired(const Struct &spiceStruct, Scope *structScope) {
-  ASTNode *node = spiceStruct.declNode;
+  auto node = spice_pointer_cast<StructDefNode *>(spiceStruct.declNode);
   assert(structScope != nullptr && structScope->type == ScopeType::STRUCT);
 
   // Abort if the struct already has a user-defined constructor
@@ -140,17 +147,24 @@ void TypeChecker::createDefaultCopyCtorIfRequired(const Struct &spiceStruct, Sco
       return;
 
     if (fieldSymbol->getType().is(TY_STRUCT)) {
-      Scope *fieldScope = fieldSymbol->getType().getBodyScope();
-      // Lookup ctor function
+      Scope *bodyScope = fieldSymbol->getType().getBodyScope();
+      Struct *fieldStruct = fieldSymbol->getType().getStruct(node);
+      // Check if we are required to call a ctor
+      const auto structDeclNode = spice_pointer_cast<StructDefNode *>(fieldStruct->declNode);
+      const bool isCtorCallRequired = bodyScope->hasRefFields() || structDeclNode->emitVTable;
+      // Lookup copy ctor function
       const ArgList args = {{thisType.toConstReference(node), false /* we always have the field as storage */}};
-      const Function *ctorFct = FunctionManager::matchFunction(fieldScope, CTOR_FUNCTION_NAME, thisType, args, {}, true, node);
+      const Function *ctorFct = FunctionManager::matchFunction(bodyScope, CTOR_FUNCTION_NAME, thisType, args, {}, true, node);
+      // If we are required to construct, but no constructor is found, we can't generate a default ctor for the outer struct
+      if (!ctorFct && isCtorCallRequired)
+        return;
       hasFieldsToCopyConstruct |= ctorFct != nullptr;
       requestRevisitIfRequired(ctorFct);
     }
   }
 
   // If we don't have any fields, that require us to do anything in the copy ctor, we can skip it
-  if (!hasFieldsToCopyConstruct)
+  if (!hasFieldsToCopyConstruct && !node->emitVTable)
     return;
 
   // Create the default copy ctor function

diff --git a/test/test-files/typechecker/structs/error-missing-no-args-ctor-ref-fields/exception.out b/test/test-files/typechecker/structs/error-missing-no-args-ctor-ref-fields/exception.out
@@ -0,0 +1,8 @@
+[Error|Compiler]:
+Unresolved soft errors: There are unresolved errors. Please fix them and recompile.
+
+[Error|Semantic] ./source.spice:7:5:
+No matching ctor found: Struct 'TypeWithVTable' misses a no-args constructor
+
+7  TypeWithVTable v;
+   ^^^^^^^^^^^^^^^^
diff --git a/test/test-files/typechecker/structs/error-missing-no-args-ctor-ref-fields/source.spice b/test/test-files/typechecker/structs/error-missing-no-args-ctor-ref-fields/source.spice
@@ -0,0 +1,8 @@
+type TypeWithVTable struct {
+    int& refField
+}
+
+f<int> main() {
+    // Types containing reference fields can't have a default constructor, thus this should fail
+    TypeWithVTable v;
+}
diff --git a/test/test-files/typechecker/structs/error-missing-no-args-ctor-vtable/exception.out b/test/test-files/typechecker/structs/error-missing-no-args-ctor-vtable/exception.out
@@ -0,0 +1,8 @@
+[Error|Compiler]:
+Unresolved soft errors: There are unresolved errors. Please fix them and recompile.
+
+[Error|Semantic] ./source.spice:13:5:
+No matching ctor found: Struct 'TypeWithVTable' misses a no-args constructor
+
+13  TypeWithVTable v;
+    ^^^^^^^^^^^^^^^^
diff --git a/test/test-files/typechecker/structs/error-missing-no-args-ctor-vtable/source.spice b/test/test-files/typechecker/structs/error-missing-no-args-ctor-vtable/source.spice
@@ -0,0 +1,14 @@
+type TypeContainingRefField struct {
+    int& refField
+}
+
+#[core.compiler.alwaysEmitVTable]
+type TypeWithVTable struct {
+    TypeContainingRefField refFieldStruct
+}
+
+f<int> main() {
+    // Can't call default ctor, because it can't be generated due to the reference field
+    // But we need to call it to trigger the generation of the vtable.
+    TypeWithVTable v;
+}