From 9f9426064502715910c7fe3e6baf2da5b614c4ff Mon Sep 17 00:00:00 2001 From: Marc Auberer Date: Fri, 17 Jan 2025 02:05:55 +0100 Subject: [PATCH] Disallow returning references to temporary values --- media/test-project/test.spice | 121 ++++++++++++------ src/exception/SemanticError.cpp | 2 + src/exception/SemanticError.h | 1 + src/irgenerator/GenImplicit.cpp | 3 + src/typechecker/OpRuleManager.cpp | 4 +- src/typechecker/TypeChecker.cpp | 10 +- std/test/lifetime-object.spice | 38 ++++++ .../structs/success-destructors3/cout.out | 1 + .../structs/success-destructors3/ir-code.ll | 53 ++++++++ .../structs/success-destructors3/source.spice | 11 ++ .../std/test/lifetime-object/cout.out | 28 ++++ .../std/test/lifetime-object/source.spice | 78 +++++++++++ .../exception.out | 5 + .../source.spice | 7 + 14 files changed, 314 insertions(+), 48 deletions(-) create mode 100644 std/test/lifetime-object.spice create mode 100644 test/test-files/irgenerator/structs/success-destructors3/cout.out create mode 100644 test/test-files/irgenerator/structs/success-destructors3/ir-code.ll create mode 100644 test/test-files/irgenerator/structs/success-destructors3/source.spice create mode 100644 test/test-files/std/test/lifetime-object/cout.out create mode 100644 test/test-files/std/test/lifetime-object/source.spice create mode 100644 test/test-files/typechecker/functions/error-function-temporary-ref-return/exception.out create mode 100644 test/test-files/typechecker/functions/error-function-temporary-ref-return/source.spice diff --git a/media/test-project/test.spice b/media/test-project/test.spice index ddf9e1e65..0b7a5f117 100644 --- a/media/test-project/test.spice +++ b/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 spawnLO() { + return LifetimeObject(); } -p MockSourceFile.ctor(string fileName, unsigned long lineCount = 0l) { - this.fileName = fileName; - this.lineCount = lineCount; -} +f main() { + // Ignored return value of ctor + printf("Ignored return value of ctor"); + { + LifetimeObject(); + } -f 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 MockSourceFile.getFileName() { - return this.fileName; -} + // Return from lambda as value + printf("Return from lambda as value:\n"); + { + const f() spawnLO = f() { + return LifetimeObject(); + }; -f 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 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() spawnLO = f() { + 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() spawnLO = f() { + 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!"); } \ No newline at end of file diff --git a/src/exception/SemanticError.cpp b/src/exception/SemanticError.cpp index 3d449271d..c708c02d2 100644 --- a/src/exception/SemanticError.cpp +++ b/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: diff --git a/src/exception/SemanticError.h b/src/exception/SemanticError.h index a91e72120..f43bd1cef 100644 --- a/src/exception/SemanticError.h +++ b/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, diff --git a/src/irgenerator/GenImplicit.cpp b/src/irgenerator/GenImplicit.cpp index 1820804bc..1d4d5a328 100644 --- a/src/irgenerator/GenImplicit.cpp +++ b/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); diff --git a/src/typechecker/OpRuleManager.cpp b/src/typechecker/OpRuleManager.cpp index 3944c973b..ea384cd79 100644 --- a/src/typechecker/OpRuleManager.cpp +++ b/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 diff --git a/src/typechecker/TypeChecker.cpp b/src/typechecker/TypeChecker.cpp index 3c416a0b9..79b57a01b 100644 --- a/src/typechecker/TypeChecker.cpp +++ b/src/typechecker/TypeChecker.cpp @@ -545,15 +545,15 @@ std::any TypeChecker::visitDeclStmt(DeclStmtNode *node) { auto rhs = std::any_cast(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(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(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); diff --git a/std/test/lifetime-object.spice b/std/test/lifetime-object.spice new file mode 100644 index 000000000..463a1c3b5 --- /dev/null +++ b/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); +} diff --git a/test/test-files/irgenerator/structs/success-destructors3/cout.out b/test/test-files/irgenerator/structs/success-destructors3/cout.out new file mode 100644 index 000000000..8ab832144 --- /dev/null +++ b/test/test-files/irgenerator/structs/success-destructors3/cout.out @@ -0,0 +1 @@ +defaulttest \ No newline at end of file diff --git a/test/test-files/irgenerator/structs/success-destructors3/ir-code.ll b/test/test-files/irgenerator/structs/success-destructors3/ir-code.ll new file mode 100644 index 000000000..3ea6829e0 --- /dev/null +++ b/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 } diff --git a/test/test-files/irgenerator/structs/success-destructors3/source.spice b/test/test-files/irgenerator/structs/success-destructors3/source.spice new file mode 100644 index 000000000..e5b09372f --- /dev/null +++ b/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 main() { + test(); + test(String("test")); +} \ No newline at end of file diff --git a/test/test-files/std/test/lifetime-object/cout.out b/test/test-files/std/test/lifetime-object/cout.out new file mode 100644 index 000000000..37ef51925 --- /dev/null +++ b/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) diff --git a/test/test-files/std/test/lifetime-object/source.spice b/test/test-files/std/test/lifetime-object/source.spice new file mode 100644 index 000000000..51f8429da --- /dev/null +++ b/test/test-files/std/test/lifetime-object/source.spice @@ -0,0 +1,78 @@ +import "std/test/lifetime-object"; + +f spawnLO() { + return LifetimeObject(); +} + +f main() { + // Ignored return value of ctor + printf("Ignored return value of ctor:\n"); + { + LifetimeObject(); + } + + // 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 + + // Return from lambda as value + printf("Return from lambda as value:\n"); + { + const f() spawnLO = f() { + return LifetimeObject(); + }; + + // 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() spawnLO = f() { + 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() spawnLO = f() { + 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 + } +} \ No newline at end of file diff --git a/test/test-files/typechecker/functions/error-function-temporary-ref-return/exception.out b/test/test-files/typechecker/functions/error-function-temporary-ref-return/exception.out new file mode 100644 index 000000000..8de77da38 --- /dev/null +++ b/test/test-files/typechecker/functions/error-function-temporary-ref-return/exception.out @@ -0,0 +1,5 @@ +[Error|Semantic] ./source.spice:2:12: +Return of temporary value: Cannot return reference to temporary value + +2 return 1.2; + ^^^ \ No newline at end of file diff --git a/test/test-files/typechecker/functions/error-function-temporary-ref-return/source.spice b/test/test-files/typechecker/functions/error-function-temporary-ref-return/source.spice new file mode 100644 index 000000000..73b9930f2 --- /dev/null +++ b/test/test-files/typechecker/functions/error-function-temporary-ref-return/source.spice @@ -0,0 +1,7 @@ +f test() { + return 1.2; +} + +f main() { + double res = test(); +} \ No newline at end of file