Disallow returning references to temporary values (#712)

media/test-project/test.spice

@@ -1,50 +1,87 @@
-import "std/data/vector";
+import "std/test/lifetime-object";
 
-import "bootstrap/util/common-util";
-import "bootstrap/source-file-intf";
-
-type MockSourceFile struct : ISourceFile {
-    string fileName
-    unsigned long lineCount = 0l
+f<LifetimeObject> spawnLO() {
+    return LifetimeObject();
 }
 
-p MockSourceFile.ctor(string fileName, unsigned long lineCount = 0l) {
-    this.fileName = fileName;
-    this.lineCount = lineCount;
-}
+f<int> main() {
+    // Ignored return value of ctor
+    printf("Ignored return value of ctor");
+    {
+        LifetimeObject();
+    }
 
-f<unsigned long> MockSourceFile.getLineCount() {
-    return this.lineCount;
-}
+    // Normal lifecycle
+    printf("Normal lifecycle:\n");
+    {
+        LifetimeObject lo = LifetimeObject(); // ctor call
+        LifetimeObject loCopy = lo; // copy ctor call
+    } // dtor calls for both lo and loCopy at end of scope
 
-f<string> MockSourceFile.getFileName() {
-    return this.fileName;
-}
+    // Return from lambda as value
+    printf("Return from lambda as value:\n");
+    {
+        const f<LifetimeObject>() spawnLO = f<LifetimeObject>() {
+            return LifetimeObject();
+        };
 
-f<int> main() {
-    // getLastFragment
-    String lastFragment = getLastFragment(String("this.is.a.test.haystack"), ".");
-    printf("%s", lastFragment);
-    assert lastFragment == "haystack";
-    lastFragment = getLastFragment(String(""), ";");
-    assert lastFragment == "";
-    lastFragment = getLastFragment(String("x-y-z-"), "-");
-    assert lastFragment == "";
-    // getCircularImportMessage
-    const MockSourceFile msf1 = MockSourceFile("file1.spice", 1l);
-    const MockSourceFile msf2 = MockSourceFile("file2.spice", 12l);
-    const MockSourceFile msf3 = MockSourceFile("file3.spice", 123l);
-    const MockSourceFile msf4 = MockSourceFile("file4.spice", 1234l);
-    Vector<const ISourceFile*> sourceFiles;
-    unsafe {
-        sourceFiles.pushBack((const ISourceFile*) &msf1);
-        sourceFiles.pushBack((const ISourceFile*) &msf2);
-        sourceFiles.pushBack((const ISourceFile*) &msf3);
-        sourceFiles.pushBack((const ISourceFile*) &msf4);
+        // No return value receiver
+        spawnLO(); // Anonymous symbol
+        // Return value receiver - value
+        LifetimeObject lo = spawnLO(); // Assigned to lo
+        // Return value receiver - const ref
+        const LifetimeObject& loConstRef = spawnLO(); // Assigned to loConstRef
+    }
+
+    // Return from lambda as reference
+    printf("Return from lambda as reference:\n");
+    {
+        LifetimeObject loOrig = LifetimeObject();
+        const f<LifetimeObject&>() spawnLO = f<LifetimeObject&>() {
+            return loOrig;
+        };
+
+        // No return value receiver
+        spawnLO(); // Anonymous symbol
+        // Return value receiver - value
+        LifetimeObject lo = spawnLO(); // Assigned to lo
+        // Return value receiver - const ref
+        const LifetimeObject& loConstRef = spawnLO(); // Assigned to loConstRef
+    }
+
+    // Return from lambda as const reference
+    printf("Return from lambda as const reference:\n");
+    {
+        LifetimeObject loOrig = LifetimeObject();
+        const f<const LifetimeObject&>() spawnLO = f<const LifetimeObject&>() {
+            return loOrig;
+        };
+
+        // No return value receiver
+        spawnLO(); // Anonymous symbol
+        // Return value receiver - value
+        LifetimeObject lo = spawnLO(); // Assigned to lo
+        // Return value receiver - const ref
+        const LifetimeObject& loConstRef = spawnLO(); // Assigned to loConstRef
+    }
+
+    // Return from function as value
+    printf("Return from function as value:\n");
+    {
+        // No return value receiver
+        spawnLO(); // Anonymous symbol
+        // Return value receiver - value
+        LifetimeObject lo = spawnLO(); // Assigned to lo
+        // Return value receiver - const ref
+        const LifetimeObject& loConstRef = spawnLO(); // Assigned to loConstRef
+    }
+
+    printf("Ternary\n");
+    {
+        bool cond = false;
+        LifetimeObject lo1 = LifetimeObject();
+        LifetimeObject lo2 = LifetimeObject();
+        LifetimeObject lo3 = cond ? lo1 : lo2;
+        // ToDo: The dtor of lo2 is called twice
     }
-    const String cim = getCircularImportMessage(sourceFiles);
-    printf("Circular error message:\n%s\n", cim);
-    const String versionInfo = buildVersionInfo();
-    printf("Version info:\n%s\n", versionInfo);
-    printf("All assertions passed!");
 }

src/exception/SemanticError.cpp

@@ -178,6 +178,8 @@ std::string SemanticError::getMessagePrefix(SemanticErrorType errorType) {
     return "Return without initialization of result variable";
   case RETURN_WITH_VALUE_IN_PROCEDURE:
     return "Return with value in procedure";
+  case RETURN_OF_TEMPORARY_VALUE:
+    return "Return of temporary value";
   case INVALID_STRUCT_INSTANTIATION:
     return "Invalid struct instantiation";
   case DYN_POINTERS_NOT_ALLOWED:

src/exception/SemanticError.h

@@ -88,6 +88,7 @@ enum SemanticErrorType : uint8_t {
   TOO_MANY_SYSCALL_ARGS,
   RETURN_WITHOUT_VALUE_RESULT,
   RETURN_WITH_VALUE_IN_PROCEDURE,
+  RETURN_OF_TEMPORARY_VALUE,
   INVALID_STRUCT_INSTANTIATION,
   DYN_POINTERS_NOT_ALLOWED,
   REF_POINTERS_ARE_NOT_ALLOWED,

src/irgenerator/GenImplicit.cpp

@@ -137,6 +137,9 @@ void IRGenerator::generateCtorOrDtorCall(const SymbolTableEntry *entry, const Fu
     structAddr = insertInBoundsGEP(thisType, thisPtr, indices);
   } else {
     structAddr = entry->getAddress();
+    // For optional parameter initializers we need this exception
+    if (!structAddr)
+      return;
   }
   assert(structAddr != nullptr);
   generateCtorOrDtorCall(structAddr, ctorOrDtor, args);

src/typechecker/OpRuleManager.cpp

@@ -27,7 +27,7 @@ QualType OpRuleManager::getAssignResultType(const ASTNode *node, const ExprResul
   // Check if we try to assign a constant value
   ensureNoConstAssign(node, lhsType, isDecl, isReturn);
 
-  // Allow pointers and arrays of the same type straight away
+  // Allow pointers and references of the same type straight away
   if (lhsType.isOneOf({TY_PTR, TY_REF}) && lhsType.matches(rhsType, false, false, true)) {
     // If we perform a heap x* = heap x* assignment, we need set the right hand side to MOVED
     if (rhs.entry && lhsType.isPtr() && lhsType.isHeap() && rhsType.removeReferenceWrapper().isPtr() && rhsType.isHeap())
@@ -125,6 +125,8 @@ QualType OpRuleManager::getAssignResultTypeCommon(const ASTNode *node, const Exp
     const bool isDeclOrReturn = isDecl || isReturn;
     if (isDeclOrReturn && !lhsType.canBind(rhsType, rhs.isTemporary()))
       throw SemanticError(node, TEMP_TO_NON_CONST_REF, "Temporary values can only be bound to const reference variables/fields");
+    if (isReturn && rhs.isTemporary())
+      throw SemanticError(node, RETURN_OF_TEMPORARY_VALUE, "Cannot return reference to temporary value");
     return lhsType;
   }
   // Allow dyn[] (empty array literal) to any array

src/typechecker/TypeChecker.cpp

@@ -545,15 +545,15 @@ std::any TypeChecker::visitDeclStmt(DeclStmtNode *node) {
     auto rhs = std::any_cast<ExprResult>(visit(node->assignExpr));
     auto [rhsTy, rhsEntry] = rhs;
 
-    // If there is an anonymous entry attached (e.g. for struct instantiation), delete it
-    if (rhsEntry != nullptr && rhsEntry->anonymous) {
+    // Visit data type
+    localVarType = std::any_cast<QualType>(visit(node->dataType));
+
+    // If there is an anonymous entry attached (e.g. for struct instantiation) and we take over ownership, delete it
+    if (!localVarType.isRef() && rhsEntry != nullptr && rhsEntry->anonymous) {
       currentScope->symbolTable.deleteAnonymous(rhsEntry->name);
       rhs.entry = rhsEntry = nullptr;
     }
 
-    // Visit data type
-    localVarType = std::any_cast<QualType>(visit(node->dataType));
-
     // Infer the type left to right if the right side is an empty array initialization
     if (rhsTy.isArrayOf(TY_DYN))
       rhsTy = QualType(localVarType);

std/test/lifetime-object.spice

@@ -0,0 +1,38 @@
+unsigned int OBJECT_COUNTER = 0;
+
+/**
+ * Object that prints to cout when one of its lifetime methods is called.
+ * This is useful for language verification and debugging purposes.
+ */
+public type LifetimeObject struct {
+    int objectNumber
+    bool ctorCalled = false
+    bool copyCtorCalled = false
+    bool dtorCalled = false
+}
+
+public p LifetimeObject.ctor(int objectNumber = ++OBJECT_COUNTER) {
+    if this.ctorCalled {
+        printf("-- LifetimeObject %d ctor was called again. This indicates a compiler bug. Please report it!\n", this.objectNumber);
+    }
+    this.objectNumber = objectNumber;
+    this.ctorCalled = true;
+    printf("-- LifetimeObject %d was created (ctor)\n", this.objectNumber);
+}
+
+public p LifetimeObject.ctor(const LifetimeObject& other) {
+    if this.copyCtorCalled {
+        printf("-- LifetimeObject %d copy ctor was called again. This indicates a compiler bug. Please report it!\n", this.objectNumber);
+    }
+    this.objectNumber = ++OBJECT_COUNTER;
+    this.copyCtorCalled = true;
+    printf("-- LifetimeObject %d was copied to LifetimeObject %d (copy ctor)\n", other.objectNumber, this.objectNumber);
+}
+
+public p LifetimeObject.dtor() {
+    if this.dtorCalled {
+        printf("-- LifetimeObject %d dtor was called again. This indicates a compiler bug. Please report it!\n", this.objectNumber);
+    }
+    this.dtorCalled = true;
+    printf("-- LifetimeObject %d (dtor)\n", this.objectNumber);
+}

test/test-files/irgenerator/structs/success-destructors3/cout.out

@@ -0,0 +1 @@
+defaulttest

test/test-files/irgenerator/structs/success-destructors3/ir-code.ll

@@ -0,0 +1,53 @@
+; ModuleID = 'source.spice'
+source_filename = "source.spice"
+
+%struct.String = type { ptr, i64, i64 }
+
+@anon.string.0 = private unnamed_addr constant [8 x i8] c"default\00", align 1
+@printf.str.0 = private unnamed_addr constant [3 x i8] c"%s\00", align 1
+@printf.str.1 = private unnamed_addr constant [3 x i8] c"%s\00", align 1
+@anon.string.1 = private unnamed_addr constant [5 x i8] c"test\00", align 1
+
+define private void @_Z4testv() {
+  %1 = alloca %struct.String, align 8
+  %t = alloca ptr, align 8
+  call void @_ZN6String4ctorEPKc(ptr noundef nonnull align 8 dereferenceable(24) %1, ptr @anon.string.0)
+  store ptr %1, ptr %t, align 8
+  %2 = load ptr, ptr %t, align 8
+  %3 = load ptr, ptr %2, align 8
+  %4 = call i32 (ptr, ...) @printf(ptr noundef @printf.str.0, ptr %3)
+  call void @_ZN6String4dtorEv(ptr %1)
+  ret void
+}
+
+declare void @_ZN6String4ctorEPKc(ptr, ptr)
+
+; Function Attrs: nofree nounwind
+declare noundef i32 @printf(ptr nocapture noundef readonly, ...) #0
+
+declare void @_ZN6String4dtorEv(ptr)
+
+define private void @_Z4testRK6String(ptr %0) {
+  %t = alloca ptr, align 8
+  store ptr %0, ptr %t, align 8
+  %2 = load ptr, ptr %t, align 8
+  %3 = load ptr, ptr %2, align 8
+  %4 = call i32 (ptr, ...) @printf(ptr noundef @printf.str.1, ptr %3)
+  ret void
+}
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @main() #1 {
+  %result = alloca i32, align 4
+  %1 = alloca %struct.String, align 8
+  store i32 0, ptr %result, align 4
+  call void @_Z4testv()
+  call void @_ZN6String4ctorEPKc(ptr noundef nonnull align 8 dereferenceable(24) %1, ptr @anon.string.1)
+  call void @_Z4testRK6String(ptr %1)
+  call void @_ZN6String4dtorEv(ptr %1)
+  %2 = load i32, ptr %result, align 4
+  ret i32 %2
+}
+
+attributes #0 = { nofree nounwind }
+attributes #1 = { noinline nounwind optnone uwtable }

test/test-files/irgenerator/structs/success-destructors3/source.spice

@@ -0,0 +1,11 @@
+// This function generates two substantiations: One with the default value and one with the given value from
+// the caller. If the default value is used, it has to be destroyed at the end of the scope. Therefore, the
+// first substantiation calls the dtor and the second one not.
+p test(const String& t = String("default")) {
+    printf("%s", t);
+}
+
+f<int> main() {
+    test();
+    test(String("test"));
+}

test/test-files/std/test/lifetime-object/cout.out

@@ -0,0 +1,28 @@
+Ignored return value of ctor:
+-- LifetimeObject 1 was created (ctor)
+-- LifetimeObject 1 (dtor)
+Normal lifecycle:
+-- LifetimeObject 2 was created (ctor)
+-- LifetimeObject 2 was copied to LifetimeObject 3 (copy ctor)
+-- LifetimeObject 3 (dtor)
+-- LifetimeObject 2 (dtor)
+Return from lambda as value:
+-- LifetimeObject 4 was created (ctor)
+-- LifetimeObject 5 was created (ctor)
+-- LifetimeObject 6 was created (ctor)
+-- LifetimeObject 6 (dtor)
+-- LifetimeObject 5 (dtor)
+-- LifetimeObject 4 (dtor)
+Return from lambda as reference:
+-- LifetimeObject 7 was created (ctor)
+-- LifetimeObject 7 (dtor)
+Return from lambda as const reference:
+-- LifetimeObject 8 was created (ctor)
+-- LifetimeObject 8 (dtor)
+Return from function as value:
+-- LifetimeObject 9 was created (ctor)
+-- LifetimeObject 10 was created (ctor)
+-- LifetimeObject 11 was created (ctor)
+-- LifetimeObject 11 (dtor)
+-- LifetimeObject 10 (dtor)
+-- LifetimeObject 9 (dtor)