Skip to content

Commit 1ce9cb7

Browse files
authored
CSharpExpressionPrinter: Recurse into operands (mono#1745)
* CSharpExpressionPrinter: Recurse into operands Recursively call `VisitExpression` on the LHS and RHS of a binary operator expression. This fixes the generation for complex default parameters involving things other than two enumeration members. * CSharpSources: Use `const` when possible Generate `const` instead of `static` members when possible. This allows generated members to be used when compile-time constants are required, such as default parameters.
1 parent 160efb4 commit 1ce9cb7

File tree

4 files changed

+39
-26
lines changed

4 files changed

+39
-26
lines changed

src/Generator/Generators/CSharp/CSharpExpressionPrinter.cs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,8 @@ public string VisitExpression(ExpressionObsolete expr)
6161
case StatementClass.BinaryOperator:
6262
var binaryOperator = (BinaryOperatorObsolete)expr;
6363

64-
var lhsResult = binaryOperator.LHS.String;
65-
if (binaryOperator.LHS.Declaration is Enumeration.Item)
66-
lhsResult = binaryOperator.LHS.Declaration.Visit(typePrinter).Type;
67-
68-
var rhsResult = binaryOperator.RHS.String;
69-
if (binaryOperator.RHS.Declaration is Enumeration.Item)
70-
rhsResult = binaryOperator.RHS.Declaration.Visit(typePrinter).Type;
64+
var lhsResult = VisitExpression(binaryOperator.LHS);
65+
var rhsResult = VisitExpression(binaryOperator.RHS);
7166

7267
return $"{lhsResult} {binaryOperator.OpcodeStr} {rhsResult}";
7368
case StatementClass.ConstructorReference:

src/Generator/Generators/CSharp/CSharpSources.cs

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -256,9 +256,9 @@ public virtual void GenerateNamespaceFunctionsAndVariables(DeclarationContext co
256256
.Any();
257257

258258
using (PushWriteBlock(BlockKind.Functions, $"public unsafe partial {(isStruct ? "struct" : "class")} {parentName}", NewLineKind.BeforeNextBlock))
259-
{
259+
{
260260
using (PushWriteBlock(BlockKind.InternalsClass, GetClassInternalHead(new Class { Name = parentName }), NewLineKind.BeforeNextBlock))
261-
{
261+
{
262262
// Generate all the internal function declarations.
263263
foreach (var function in context.Functions)
264264
{
@@ -301,7 +301,7 @@ template.OriginalNamespace is Class &&
301301
template.Name);
302302

303303
using (PushWriteBlock(BlockKind.Namespace, namespaceName, NewLineKind.BeforeNextBlock))
304-
{
304+
{
305305
var generated = GetGeneratedClasses(template, specializations);
306306

307307
foreach (var nestedTemplate in template.Classes.Where(
@@ -1615,19 +1615,27 @@ private void GenerateVariable(Class @class, Variable variable)
16151615
var variableType = variable.Type.Visit(TypePrinter);
16161616
TypePrinter.PopMarshalKind();
16171617

1618-
var signature = $"public static {variableType} {variable.Name}";
1618+
bool hasInitializer = variable.Initializer != null && !string.IsNullOrWhiteSpace(variable.Initializer.String);
16191619

1620-
if (variable.Initializer != null && !string.IsNullOrWhiteSpace(variable.Initializer.String))
1621-
GeneratePropertyGetterForVariableWithInitializer(variable, signature);
1620+
if (hasInitializer && variable.QualifiedType.Qualifiers.IsConst &&
1621+
(variable.Type.Desugar() is BuiltinType || variableType.ToString() == "string"))
1622+
Write($"public const {variableType} {variable.Name} = {variable.Initializer.String};");
16221623
else
16231624
{
1624-
using (WriteBlock(signature))
1625+
var signature = $"public static {variableType} {variable.Name}";
1626+
1627+
if (hasInitializer)
1628+
GeneratePropertyGetterForVariableWithInitializer(variable, signature);
1629+
else
16251630
{
1626-
GeneratePropertyGetter(variable, @class);
1631+
using (WriteBlock(signature))
1632+
{
1633+
GeneratePropertyGetter(variable, @class);
16271634

1628-
if (!variable.QualifiedType.Qualifiers.IsConst &&
1629-
!(variable.Type.Desugar() is ArrayType))
1630-
GeneratePropertySetter(variable, @class);
1635+
if (!variable.QualifiedType.Qualifiers.IsConst &&
1636+
!(variable.Type.Desugar() is ArrayType))
1637+
GeneratePropertySetter(variable, @class);
1638+
}
16311639
}
16321640
}
16331641

@@ -1780,7 +1788,7 @@ public void GenerateVTable(Class @class)
17801788
return __vtables;
17811789
}}
17821790
1783-
set {{
1791+
set {{
17841792
__vtables = value;
17851793
}}", trimIndentation: true);
17861794
}
@@ -2296,13 +2304,13 @@ private void GenerateDisposeMethods(Class @class)
22962304
// Normally, calling the native dtor should be controlled by whether or not we
22972305
// we own the underlying instance. (i.e. Helpers.OwnsNativeInstanceIdentifier).
22982306
// However, there are 2 situations when the caller needs to have direct control
2299-
//
2307+
//
23002308
// 1. When we have a virtual dtor on the native side we detour the vtable entry
23012309
// even when we don't own the underlying native instance. I think we do this
23022310
// so that the managed side can null out the __Instance pointer and remove the
23032311
// instance from the NativeToManagedMap. Of course, this is somewhat half-hearted
23042312
// since we can't/don't do this when there's no virtual dtor available to detour.
2305-
// Anyway, we must be able to call the native dtor in this case even if we don't
2313+
// Anyway, we must be able to call the native dtor in this case even if we don't
23062314
/// own the underlying native instance.
23072315
//
23082316
// 2. When we we pass a disposable object to a function "by value" then the callee
@@ -2313,7 +2321,7 @@ private void GenerateDisposeMethods(Class @class)
23132321
// ....
23142322
// compiler generates call to f.dtor() at the end of function
23152323
// }
2316-
//
2324+
//
23172325
// IDisposable.Dispose() and Object.Finalize() set callNativeDtor = Helpers.OwnsNativeInstanceIdentifier
23182326
WriteLine("if (callNativeDtor)");
23192327
if (@class.IsDependent || dtor.IsVirtual)
@@ -2334,7 +2342,7 @@ c is ClassTemplateSpecialization ?
23342342
}
23352343

23362344
// If we have any fields holding references to unmanaged memory allocated here, free the
2337-
// referenced memory. Don't rely on testing if the field's IntPtr is IntPtr.Zero since
2345+
// referenced memory. Don't rely on testing if the field's IntPtr is IntPtr.Zero since
23382346
// unmanaged memory isn't always initialized and/or a reference may be owned by the
23392347
// native side.
23402348
//
@@ -2540,7 +2548,7 @@ private void Generate__CopyValue(Class @class, string @internal)
25402548
using (WriteBlock($"private static void* __CopyValue({@internal} native)"))
25412549
{
25422550
var copyCtorMethod = @class.Methods.FirstOrDefault(method => method.IsCopyConstructor);
2543-
2551+
25442552
if (@class.HasNonTrivialCopyConstructor && copyCtorMethod != null && copyCtorMethod.IsGenerated)
25452553
{
25462554
// Allocate memory for a new native object and call the ctor.
@@ -2848,7 +2856,7 @@ private void GenerateEquals(Class @class)
28482856
private void GenerateGetHashCode(Class @class)
28492857
{
28502858
using (WriteBlock("public override int GetHashCode()"))
2851-
{
2859+
{
28522860
if (!@class.IsRefType)
28532861
WriteLine($"return {Helpers.InstanceIdentifier}.GetHashCode();");
28542862
else
@@ -2999,7 +3007,7 @@ private void GenerateClassConstructor(Method method, Class @class)
29993007
// Copy any string references owned by the source to the new instance so we
30003008
// don't have to ref count them.
30013009
// If there is no property or no setter then this instance can never own the native
3002-
// memory. Worry about the case where there's only a setter (write-only) when we
3010+
// memory. Worry about the case where there's only a setter (write-only) when we
30033011
// understand the use case and how it can occur.
30043012
foreach (var prop in @class.GetConstCharFieldProperties())
30053013
{

tests/dotnet/Common/Common.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1278,3 +1278,7 @@ extern "C"
12781278
void DLL_API FunctionWithFlagsAsDefaultParameter(int defaultParam)
12791279
{
12801280
}
1281+
1282+
void DLL_API FunctionWithConstFlagsAsDefaultParameter(int defaultParam)
1283+
{
1284+
}

tests/dotnet/Common/Common.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1569,3 +1569,9 @@ extern "C"
15691569
} // extern "C"
15701570

15711571
void DLL_API FunctionWithFlagsAsDefaultParameter(int defaultParam = A | B);
1572+
1573+
const int ConstFlag1 = 1;
1574+
const int ConstFlag2 = 2;
1575+
const int ConstFlag3 = 4;
1576+
1577+
void DLL_API FunctionWithConstFlagsAsDefaultParameter(int defaultParam = ConstFlag1 | ConstFlag2 | ConstFlag3);

0 commit comments

Comments
 (0)