From 929e4e630a5e2151425bc89b67adec48b0a15909 Mon Sep 17 00:00:00 2001 From: Jacek Sieka Date: Sat, 18 Jan 2025 17:42:00 +1300 Subject: [PATCH] Simplify delete The subclassing structure in `miqt` is such that nothing ever inherits from `MiqtVirtual` - also, for `MiqtVirtual` to work correctly when Qt deletes an instance part of a tree of widgets, the technique can only be used with types that already have a virtual destructor. Therefore: * make `MiqtVirtual` final to ensure nothing inherits from it by accident * remove `virtual` inheritance of its base class - since there is no MI invonved and no further inheritance, there can also not be any diamond inheritance structures - removing `virtual` makes inheritance a bit cheaper * remove `isSubclass` from `delete` function - C++ already calls the most inherited destructor * mark destructor `override` to verify said assumption --- cmd/genbindings/emitcabi.go | 16 +++++++--------- cmd/genbindings/emitgo.go | 2 +- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/cmd/genbindings/emitcabi.go b/cmd/genbindings/emitcabi.go index c1c16064..32f84bae 100644 --- a/cmd/genbindings/emitcabi.go +++ b/cmd/genbindings/emitcabi.go @@ -756,7 +756,7 @@ extern "C" { // delete if c.CanDelete { - ret.WriteString(fmt.Sprintf("void %s_Delete(%s* self, bool isSubclass);\n", methodPrefixName, methodPrefixName)) + ret.WriteString(fmt.Sprintf("void %s_Delete(%s* self);\n", methodPrefixName, methodPrefixName)) } ret.WriteString("\n") @@ -865,7 +865,7 @@ extern "C" { overriddenClassName := "MiqtVirtual" + strings.Replace(cppClassName, `::`, ``, -1) - ret.WriteString("class " + overriddenClassName + " : public virtual " + cppClassName + " {\n" + + ret.WriteString("class " + overriddenClassName + " final : public " + cppClassName + " {\n" + "public:\n" + "\n", ) @@ -885,7 +885,7 @@ extern "C" { ) } else { ret.WriteString( - "\tvirtual ~" + overriddenClassName + "() = default;\n" + + "\tvirtual ~" + overriddenClassName + "() override = default;\n" + "\n", ) } @@ -1217,14 +1217,12 @@ extern "C" { } // Delete + // If we subclassed, our class destructor is always virtual. Therefore + // we can delete from the self ptr without any dynamic_cast<> if c.CanDelete { ret.WriteString( - "void " + methodPrefixName + "_Delete(" + methodPrefixName + "* self, bool isSubclass) {\n" + - "\tif (isSubclass) {\n" + - "\t\tdelete dynamic_cast<" + cppClassName + "*>( self );\n" + - "\t} else {\n" + - "\t\tdelete self;\n" + - "\t}\n" + + "void " + methodPrefixName + "_Delete(" + methodPrefixName + "* self) {\n" + + "\tdelete self;\n" + "}\n" + "\n", ) diff --git a/cmd/genbindings/emitgo.go b/cmd/genbindings/emitgo.go index d90316da..cb5a6774 100644 --- a/cmd/genbindings/emitgo.go +++ b/cmd/genbindings/emitgo.go @@ -1074,7 +1074,7 @@ import "C" ret.WriteString(` // Delete this object from C++ memory. func (this *` + goClassName + `) Delete() { - C.` + goClassName + `_Delete(this.h, C.bool(this.isSubclass)) + C.` + goClassName + `_Delete(this.h) } // GoGC adds a Go Finalizer to this pointer, so that it will be deleted