From e4a40d0eed1f8827b8fdaf3816414d9aee16d591 Mon Sep 17 00:00:00 2001 From: Dimitar Dobrev Date: Thu, 5 Nov 2015 03:00:26 +0200 Subject: [PATCH] Revert "Fixed a crash when calling the dtor of an abstract type with MinGW." This reverts commit 67e6e1204cb653093985430e0e8bee9b55b476f7. Signed-off-by: Dimitar Dobrev --- .../Generators/CSharp/CSharpTextTemplate.cs | 107 +++++++----------- tests/Common/Common.Tests.cs | 15 --- tests/Common/Common.cpp | 8 -- tests/Common/Common.h | 8 -- 4 files changed, 44 insertions(+), 94 deletions(-) diff --git a/src/Generator/Generators/CSharp/CSharpTextTemplate.cs b/src/Generator/Generators/CSharp/CSharpTextTemplate.cs index 1bae205e..075e22e8 100644 --- a/src/Generator/Generators/CSharp/CSharpTextTemplate.cs +++ b/src/Generator/Generators/CSharp/CSharpTextTemplate.cs @@ -512,11 +512,7 @@ namespace CppSharp.Generators.CSharp method.SynthKind != FunctionSynthKind.AdjustedMethod) return; - if (method.IsProxy || - (method.IsVirtual && !method.IsOperator && - // virtual destructors in abstract classes may lack a pointer in the v-table - // so they have to be called by symbol and therefore not ignored - !(method.IsDestructor && @class.IsAbstract))) + if (method.IsProxy || (method.IsVirtual && !method.IsOperator)) return; functions.Add(method); @@ -1394,7 +1390,7 @@ namespace CppSharp.Generators.CSharp { if (method.IsDestructor) { - WriteLine("{0}.DisposeImpl(false);", Helpers.TargetIdentifier); + WriteLine("{0}.Dispose(false);", Helpers.TargetIdentifier); return; } @@ -1702,13 +1698,7 @@ namespace CppSharp.Generators.CSharp var dtor = @class.Destructors.FirstOrDefault(d => d.Access != AccessSpecifier.Private && d.IsVirtual); var baseDtor = @class.BaseClass == null ? null : @class.BaseClass.Destructors.FirstOrDefault(d => !d.IsVirtual); - if (ShouldGenerateClassNativeField(@class) || (dtor != null && baseDtor != null) || - // virtual destructors in abstract classes may lack a pointer in the v-table - // so they have to be called by symbol; thus we need an explicit Dispose override - @class.IsAbstract || - // if the base type is abstract and the current type not, - // we need the regular v-table call so we have to override Dispose again - (!@class.IsAbstractImpl && @class.BaseClass != null && @class.BaseClass.IsAbstract)) + if (ShouldGenerateClassNativeField(@class) || (dtor != null && baseDtor != null)) GenerateDisposeMethods(@class); } } @@ -1732,6 +1722,21 @@ namespace CppSharp.Generators.CSharp { var hasBaseClass = @class.HasBaseClass && @class.BaseClass.IsRefType; + // Generate the IDispose Dispose() method. + if (!hasBaseClass) + { + PushBlock(CSharpBlockKind.Method); + WriteLine("public void Dispose()"); + WriteStartBraceIndent(); + + WriteLine("Dispose(disposing: true);"); + if (Options.GenerateFinalizers) + WriteLine("GC.SuppressFinalize(this);"); + + WriteCloseBraceIndent(); + PopBlock(NewLineKind.BeforeNextBlock); + } + // Generate Dispose(bool) method PushBlock(CSharpBlockKind.Method); if (@class.IsValueType) @@ -1746,43 +1751,6 @@ namespace CppSharp.Generators.CSharp WriteLine("void Dispose(bool disposing)"); WriteStartBraceIndent(); - - var dtor = @class.Destructors.FirstOrDefault(); - if (dtor != null && dtor.Access != AccessSpecifier.Private && - @class.HasNonTrivialDestructor && !dtor.IsPure) - { - NativeLibrary library; - if (!Options.CheckSymbols || - Driver.Symbols.FindLibraryBySymbol(dtor.Mangled, out library)) - { - if (dtor.IsVirtual && !@class.IsAbstract) - GenerateVirtualFunctionCall(dtor, @class, true); - else - GenerateInternalFunctionCall(dtor); - } - } - - WriteCloseBraceIndent(); - PopBlock(NewLineKind.BeforeNextBlock); - - if (hasBaseClass) return; - - // Generate the IDispose Dispose() method. - PushBlock(CSharpBlockKind.Method); - WriteLine("public void Dispose()"); - WriteStartBraceIndent(); - - WriteLine("DisposeImpl(disposing: true);"); - if (Options.GenerateFinalizers) - WriteLine("GC.SuppressFinalize(this);"); - - WriteCloseBraceIndent(); - PopBlock(NewLineKind.BeforeNextBlock); - - PushBlock(CSharpBlockKind.Method); - WriteLine("protected void DisposeImpl(bool disposing)"); - WriteStartBraceIndent(); - WriteLine("if (!{0} && disposing)", Helpers.OwnsNativeInstanceIdentifier); WriteLineIndent("throw new global::System.InvalidOperationException" + "(\"Managed instances owned by native code cannot be disposed of.\");"); @@ -1812,7 +1780,20 @@ namespace CppSharp.Generators.CSharp } } - WriteLine("Dispose(disposing: true);"); + var dtor = @class.Destructors.FirstOrDefault(); + if (dtor != null && dtor.Access != AccessSpecifier.Private && + @class.HasNonTrivialDestructor && !dtor.IsPure) + { + NativeLibrary library; + if (!Options.CheckSymbols || + Driver.Symbols.FindLibraryBySymbol(dtor.Mangled, out library)) + { + if (dtor.IsVirtual) + GenerateVirtualFunctionCall(dtor, @class); + else + GenerateInternalFunctionCall(dtor); + } + } WriteLine("if ({0})", Helpers.OwnsNativeInstanceIdentifier); WriteLineIndent("Marshal.FreeHGlobal({0});", Helpers.InstanceIdentifier); @@ -2252,11 +2233,14 @@ namespace CppSharp.Generators.CSharp private static AccessSpecifier GetValidPropertyAccess(Property property) { - if (property.Access == AccessSpecifier.Public) - return AccessSpecifier.Public; - return property.IsOverride - ? ((Class) property.Namespace).GetBaseProperty(property).Access - : property.Access; + switch (property.Access) + { + case AccessSpecifier.Public: + return AccessSpecifier.Public; + default: + return property.IsOverride ? + ((Class) property.Namespace).GetBaseProperty(property).Access : property.Access; + } } private void GenerateVirtualPropertyCall(Function method, Class @class, @@ -2266,8 +2250,7 @@ namespace CppSharp.Generators.CSharp method.SynthKind != FunctionSynthKind.AbstractImplCall && @class.HasNonAbstractBaseProperty(property)) { - WriteLine(parameters == null ? - "return base.{0};" : "base.{0} = value;", property.Name); + WriteLine(parameters == null ? "return base.{0};" : "base.{0} = value;", property.Name); } else { @@ -2277,10 +2260,9 @@ namespace CppSharp.Generators.CSharp } } - private void GenerateVirtualFunctionCall(Method method, Class @class, - bool forceVirtualCall = false) + private void GenerateVirtualFunctionCall(Method method, Class @class) { - if (!forceVirtualCall && method.IsOverride && !method.IsPure && + if (method.IsOverride && !method.IsPure && method.SynthKind != FunctionSynthKind.AbstractImplCall && @class.HasNonAbstractBaseMethod(method)) { @@ -2299,8 +2281,7 @@ namespace CppSharp.Generators.CSharp { var virtualCallBuilder = new StringBuilder(); var i = VTables.GetVTableIndex(function.OriginalFunction ?? function, @class); - virtualCallBuilder.AppendFormat( - "var {0} = *(void**) ((IntPtr) __OriginalVTables[0] + {1} * {2});", + virtualCallBuilder.AppendFormat("var {0} = *(void**) ((IntPtr) __OriginalVTables[0] + {1} * {2});", Helpers.SlotIdentifier, i, Driver.TargetInfo.PointerWidth / 8); virtualCallBuilder.AppendLine(); diff --git a/tests/Common/Common.Tests.cs b/tests/Common/Common.Tests.cs index 3a1e4d72..7b4161cf 100644 --- a/tests/Common/Common.Tests.cs +++ b/tests/Common/Common.Tests.cs @@ -555,20 +555,5 @@ public class CommonTests : GeneratorTestFixture Assert.That(hasProblematicFields.c, Is.EqualTo('a')); } } - - [Test] - public void TestDisposingCustomDerivedFromVirtual() - { - using (new CustomDerivedFromVirtual()) - { - } - } - - private class CustomDerivedFromVirtual : AbstractWithVirtualDtor - { - public override void @abstract() - { - } - } } \ No newline at end of file diff --git a/tests/Common/Common.cpp b/tests/Common/Common.cpp index 1e7b3424..2b44ba6a 100644 --- a/tests/Common/Common.cpp +++ b/tests/Common/Common.cpp @@ -555,11 +555,3 @@ int OverridesNonDirectVirtual::retInt() { return 3; } - -AbstractWithVirtualDtor::AbstractWithVirtualDtor() -{ -} - -AbstractWithVirtualDtor::~AbstractWithVirtualDtor() -{ -} diff --git a/tests/Common/Common.h b/tests/Common/Common.h index 3148d1bf..af74f48f 100644 --- a/tests/Common/Common.h +++ b/tests/Common/Common.h @@ -933,11 +933,3 @@ AbstractTemplate::AbstractTemplate() and therefore %HashBase pointers should never be used. */ class DLL_API TestComments {}; - -class DLL_API AbstractWithVirtualDtor -{ -public: - AbstractWithVirtualDtor(); - virtual ~AbstractWithVirtualDtor(); - virtual void abstract() = 0; -};