Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 3a98968

Browse files
JonHannaOmarTawfik
authored andcommitted
Have Microsoft.CSharp pick correct default for optional MarshalAs(UnmanagedType.IDispatch) (#25508)
* Use null as default for optional parameters that marshal as IDispatch Fixes #25507 * Don't allocate six arrays for empty parameters in MethodOrPropertySymbol * More tests for default parameter value handling * Remove dead path from SetParameterAttributes Looks for default decimal value, but we've already looked for that via DecimalConstantAttribute. * Remove dead branch in handling of assigning Type.Missing Called if a parameter type which is known to be typeof(object) is typeof(Missing), which clearly can't happen. (And if it did the result would be a harmless cast from Missing to Missing anyway). * More tests for optional arguments Default enums & structs and optionals without defaults. * Assert MethodOrPropertySymbol._Params is set only once.
1 parent 016659c commit 3a98968

File tree

5 files changed

+533
-26
lines changed

5 files changed

+533
-26
lines changed

src/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/GroupToArgsBinder.cs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -586,15 +586,7 @@ private static Expr GenerateOptionalArgument(
586586
FieldSymbol field = symbolLoader.LookupAggMember(name, agg, symbmask_t.MASK_FieldSymbol) as FieldSymbol;
587587
FieldWithType fwt = new FieldWithType(field, agg.getThisType());
588588
ExprField exprField = exprFactory.CreateField(agg.getThisType(), null, fwt, false);
589-
590-
if (agg.getThisType() != type)
591-
{
592-
optionalArgument = exprFactory.CreateCast(type, exprField);
593-
}
594-
else
595-
{
596-
optionalArgument = exprField;
597-
}
589+
optionalArgument = exprFactory.CreateCast(type, exprField);
598590
}
599591
}
600592
else

src/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/Semantics/Symbols/MethodOrPropertySymbol.cs

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System;
56
using System.Collections.Generic;
67
using System.Diagnostics;
78
using System.Runtime.InteropServices;
@@ -59,16 +60,27 @@ public TypeArray Params
5960
}
6061
set
6162
{
62-
// Should only be set once!
63+
Debug.Assert(_Params == null, "Should only be set once");
6364
_Params = value;
64-
_optionalParameterIndex = new bool[_Params.Count];
65-
_defaultParameterIndex = new bool[_Params.Count];
66-
_defaultParameters = new ConstVal[_Params.Count];
67-
_defaultParameterConstValTypes = new CType[_Params.Count];
68-
_marshalAsIndex = new bool[_Params.Count];
69-
_marshalAsBuffer = new UnmanagedType[_Params.Count];
65+
int count = value.Count;
66+
if (count == 0)
67+
{
68+
_optionalParameterIndex = _defaultParameterIndex = _marshalAsIndex = Array.Empty<bool>();
69+
_defaultParameters = Array.Empty<ConstVal>();
70+
_defaultParameterConstValTypes = Array.Empty<CType>();
71+
_marshalAsBuffer = Array.Empty<UnmanagedType>();
72+
}
73+
else
74+
{
75+
_optionalParameterIndex = new bool[count];
76+
_defaultParameterIndex = new bool[count];
77+
_defaultParameters = new ConstVal[count];
78+
_defaultParameterConstValTypes = new CType[count];
79+
_marshalAsIndex = new bool[count];
80+
_marshalAsBuffer = new UnmanagedType[count];
81+
}
7082
}
71-
} // array of cParams parameter types.
83+
}
7284

7385
public MethodOrPropertySymbol()
7486
{
@@ -157,14 +169,13 @@ private UnmanagedType GetMarshalAsParameterValue(int index)
157169

158170
public bool MarshalAsObject(int index)
159171
{
160-
UnmanagedType marshalAsType = default(UnmanagedType);
161-
162172
if (IsMarshalAsParameter(index))
163173
{
164-
marshalAsType = GetMarshalAsParameterValue(index);
174+
UnmanagedType marshalAsType = GetMarshalAsParameterValue(index);
175+
return marshalAsType == UnmanagedType.Interface || marshalAsType == UnmanagedType.IUnknown || marshalAsType == UnmanagedType.IDispatch;
165176
}
166177

167-
return marshalAsType == UnmanagedType.Interface || marshalAsType == UnmanagedType.IUnknown;
178+
return false;
168179
}
169180

170181
public AggregateSymbol getClass() => parent as AggregateSymbol;

src/Microsoft.CSharp/src/Microsoft/CSharp/RuntimeBinder/SymbolTable.cs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1620,6 +1620,7 @@ private void SetParameterAttributes(MethodOrPropertySymbol methProp, ParameterIn
16201620
{
16211621
object defValue = parameter.DefaultValue;
16221622
#endif
1623+
Debug.Assert(Type.GetTypeCode(defValue.GetType()) != TypeCode.Decimal); // Handled above
16231624
switch (Type.GetTypeCode(defValue.GetType()))
16241625
{
16251626

@@ -1653,11 +1654,6 @@ private void SetParameterAttributes(MethodOrPropertySymbol methProp, ParameterIn
16531654
cvType = _semanticChecker.SymbolLoader.GetPredefindType(PredefinedType.PT_DOUBLE);
16541655
break;
16551656

1656-
case TypeCode.Decimal:
1657-
cv = ConstVal.Get((decimal)defValue);
1658-
cvType = _semanticChecker.SymbolLoader.GetPredefindType(PredefinedType.PT_DECIMAL);
1659-
break;
1660-
16611657
case TypeCode.Char:
16621658
cv = ConstVal.Get((char)defValue);
16631659
cvType = _semanticChecker.SymbolLoader.GetPredefindType(PredefinedType.PT_CHAR);

0 commit comments

Comments
 (0)