Skip to content

Commit 6a0bdc1

Browse files
ddobrevddobrev
committed
Delete the C++ copy when returning by value (#1623)
Fixes #1600. Signed-off-by: Dimitar Dobrev <[email protected]>
1 parent 1d162eb commit 6a0bdc1

File tree

6 files changed

+93
-11
lines changed

6 files changed

+93
-11
lines changed

‎src/Generator/Generators/CSharp/CSharpMarshal.cs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,48 @@ public override bool VisitClassDecl(Class @class)
282282
if (returnType.IsAddress())
283283
Context.Return.Write(HandleReturnedPointer(@class, qualifiedClass));
284284
else
285-
Context.Return.Write($"{qualifiedClass}.{Helpers.CreateInstanceIdentifier}({Context.ReturnVarName})");
285+
{
286+
if (Context.MarshalKind == MarshalKind.NativeField ||
287+
Context.MarshalKind == MarshalKind.Variable ||
288+
!originalClass.HasNonTrivialDestructor)
289+
{
290+
Context.Return.Write($"{qualifiedClass}.{Helpers.CreateInstanceIdentifier}({Context.ReturnVarName})");
291+
}
292+
else
293+
{
294+
Context.Before.WriteLine($@"var __{Context.ReturnVarName} = {
295+
qualifiedClass}.{Helpers.CreateInstanceIdentifier}({Context.ReturnVarName});");
296+
Method dtor = originalClass.Destructors.First();
297+
if (dtor.IsVirtual)
298+
{
299+
var i = VTables.GetVTableIndex(dtor);
300+
int vtableIndex = 0;
301+
if (Context.Context.ParserOptions.IsMicrosoftAbi)
302+
vtableIndex = @class.Layout.VFTables.IndexOf(@class.Layout.VFTables.First(
303+
v => v.Layout.Components.Any(c => c.Method == dtor)));
304+
Context.Before.WriteLine($@"var __vtables = new IntPtr[] {{ {
305+
string.Join(", ", originalClass.Layout.VTablePointers.Select(
306+
x => $" * (IntPtr*) ({ Helpers.InstanceIdentifier} + {x.Offset})"))} }};");
307+
Context.Before.WriteLine($"var __slot = *(IntPtr*) (__vtables[{vtableIndex}] + {i} * sizeof(IntPtr));");
308+
Context.Before.Write($"Marshal.GetDelegateForFunctionPointer<{dtor.FunctionType}>(__slot)({Helpers.InstanceIdentifier}");
309+
if (dtor.GatherInternalParams(Context.Context.ParserOptions.IsItaniumLikeAbi).Count > 1)
310+
{
311+
Context.Before.WriteLine(", 0");
312+
}
313+
Context.Before.WriteLine(");");
314+
}
315+
else
316+
{
317+
string suffix = string.Empty;
318+
var specialization = @class as ClassTemplateSpecialization;
319+
if (specialization != null)
320+
suffix = Helpers.GetSuffixFor(specialization);
321+
Context.Before.WriteLine($@"{typePrinter.PrintNative(originalClass)}.dtor{
322+
suffix}(new {typePrinter.IntPtrType}(&{Context.ReturnVarName}));");
323+
}
324+
Context.Return.Write($"__{Context.ReturnVarName}");
325+
}
326+
}
286327

287328
return true;
288329
}

‎src/Generator/Generators/CSharp/CSharpSources.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1334,8 +1334,15 @@ private void GenerateFunctionGetter(Class @class, Property property)
13341334
AddBlock(new Block(BlockKind.Unreachable));
13351335
return;
13361336
}
1337-
GenerateFunctionInProperty(@class, actualProperty.GetMethod, actualProperty,
1338-
property.QualifiedType);
1337+
QualifiedType type = default;
1338+
if (actualProperty != property ||
1339+
// indexers
1340+
(property.QualifiedType.Type.IsPrimitiveType() &&
1341+
actualProperty.GetMethod.ReturnType.Type.IsPointerToPrimitiveType()))
1342+
{
1343+
type = property.QualifiedType;
1344+
}
1345+
GenerateFunctionInProperty(@class, actualProperty.GetMethod, actualProperty, type);
13391346
}
13401347

13411348
private static Property GetActualProperty(Property property, Class c)
@@ -2180,10 +2187,8 @@ public void GenerateClassConstructors(Class @class)
21802187

21812188
// ensure any virtual dtor in the chain is called
21822189
var dtor = @class.Destructors.FirstOrDefault(d => d.Access != AccessSpecifier.Private);
2183-
var baseDtor = @class.BaseClass == null ? null :
2184-
@class.BaseClass.Destructors.FirstOrDefault(d => !d.IsVirtual);
21852190
if (ShouldGenerateClassNativeField(@class) ||
2186-
((dtor != null && (dtor.IsVirtual || @class.HasNonTrivialDestructor)) && baseDtor != null) ||
2191+
(dtor != null && (dtor.IsVirtual || @class.HasNonTrivialDestructor)) ||
21872192
// virtual destructors in abstract classes may lack a pointer in the v-table
21882193
// so they have to be called by symbol; thus we need an explicit Dispose override
21892194
@class.IsAbstract)
@@ -3464,11 +3469,6 @@ public void GenerateInternalFunction(Function function)
34643469
if (function.IsPure)
34653470
return;
34663471

3467-
if (function.OriginalName == "system_do_it")
3468-
{
3469-
System.Diagnostics.Debugger.Break();
3470-
}
3471-
34723472
PushBlock(BlockKind.InternalsClassMethod);
34733473
var callConv = function.CallingConvention.ToInteropCallConv();
34743474

‎tests/CSharp/CSharpTemplates.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -794,6 +794,7 @@ template class DLL_API Base<int>;
794794
template class DLL_API DependentValueFields<int>;
795795
template class DLL_API DependentValueFields<int*>;
796796
template class DLL_API DependentValueFields<float>;
797+
template class DLL_API DependentValueFields<double>;
797798
template class DLL_API DependentPointerFields<float>;
798799
template class DLL_API VirtualTemplate<int>;
799800
template class DLL_API VirtualTemplate<bool>;

‎tests/Common/Common.Tests.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1127,6 +1127,15 @@ public void TestConflictName()
11271127
}
11281128
}
11291129

1130+
[Test]
1131+
public void TestReturnByValueWithReturnParam()
1132+
{
1133+
using (var returnByValueWithReturnParam = ReturnByValueWithReturnParamFactory.Generate())
1134+
{
1135+
Assert.That(returnByValueWithReturnParam.UseCount, Is.EqualTo(1));
1136+
}
1137+
}
1138+
11301139
[Test, Ignore("This was exposed by another bug and doesn't work yet.")]
11311140
public void TestFreeFunctionReturnByValue()
11321141
{

‎tests/Common/Common.cpp

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,19 @@ HasPropertyNamedAsParent::~HasPropertyNamedAsParent()
13361336
{
13371337
}
13381338

1339+
ReturnByValueWithReturnParam::ReturnByValueWithReturnParam() {}
1340+
1341+
ReturnByValueWithReturnParam::ReturnByValueWithReturnParam(const ReturnByValueWithReturnParam& other) : _ptr(other._ptr) {}
1342+
1343+
ReturnByValueWithReturnParam::~ReturnByValueWithReturnParam() {}
1344+
1345+
int ReturnByValueWithReturnParam::getUseCount() { return _ptr.use_count(); }
1346+
1347+
ReturnByValueWithReturnParam ReturnByValueWithReturnParamFactory::generate()
1348+
{
1349+
return ReturnByValueWithReturnParam();
1350+
}
1351+
13391352
void integerOverload(int i)
13401353
{
13411354
}

‎tests/Common/Common.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,6 +1355,24 @@ class DLL_API HasPropertyNamedAsParent
13551355
int hasPropertyNamedAsParent;
13561356
};
13571357

1358+
class DLL_API ReturnByValueWithReturnParam
1359+
{
1360+
public:
1361+
ReturnByValueWithReturnParam();
1362+
ReturnByValueWithReturnParam(const ReturnByValueWithReturnParam& other);
1363+
~ReturnByValueWithReturnParam();
1364+
int getUseCount();
1365+
1366+
private:
1367+
std::shared_ptr<int> _ptr = std::shared_ptr<int>(new int[1]);
1368+
};
1369+
1370+
class DLL_API ReturnByValueWithReturnParamFactory
1371+
{
1372+
public:
1373+
static ReturnByValueWithReturnParam generate();
1374+
};
1375+
13581376
template<typename T> void TemplatedFunction(T type)
13591377
{
13601378

0 commit comments

Comments
 (0)