Skip to content

Commit

Permalink
Disallow dynamic calls on ref struct receivers (#72674)
Browse files Browse the repository at this point in the history
Fixes #72606. Note that the behavior in the bug has slightly changed from the sharplab build used in the repro due to #71421.
  • Loading branch information
333fred authored Apr 5, 2024
1 parent b07ea36 commit 7f05bbb
Show file tree
Hide file tree
Showing 22 changed files with 814 additions and 21 deletions.
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6326,7 +6326,7 @@ BoundExpression bindCollectionInitializerElementAddMethod(

if (implicitReceiver.Type.IsDynamic())
{
var hasErrors = ReportBadDynamicArguments(elementInitializer, boundElementInitializerExpressions, refKinds: default, diagnostics, queryClause: null);
var hasErrors = ReportBadDynamicArguments(elementInitializer, implicitReceiver, boundElementInitializerExpressions, refKinds: default, diagnostics, queryClause: null);

return new BoundDynamicCollectionElementInitializer(
elementInitializer,
Expand Down Expand Up @@ -6605,7 +6605,7 @@ protected BoundExpression BindClassCreationExpression(
var argArray = BuildArgumentsForDynamicInvocation(analyzedArguments, diagnostics);
var refKindsArray = analyzedArguments.RefKinds.ToImmutableOrNull();

hasErrors &= ReportBadDynamicArguments(node, argArray, refKindsArray, diagnostics, queryClause: null);
hasErrors &= ReportBadDynamicArguments(node, receiver: null, argArray, refKindsArray, diagnostics, queryClause: null);

BoundObjectInitializerExpressionBase boundInitializerOpt;
boundInitializerOpt = MakeBoundInitializerOpt(typeNode, type, initializerSyntaxOpt, initializerTypeOpt, diagnostics);
Expand Down Expand Up @@ -9659,7 +9659,7 @@ private BoundExpression BindDynamicIndexer(
var argArray = BuildArgumentsForDynamicInvocation(arguments, diagnostics);
var refKindsArray = arguments.RefKinds.ToImmutableOrNull();

hasErrors &= ReportBadDynamicArguments(syntax, argArray, refKindsArray, diagnostics, queryClause: null);
hasErrors &= ReportBadDynamicArguments(syntax, receiver, argArray, refKindsArray, diagnostics, queryClause: null);

return new BoundDynamicIndexerAccess(
syntax,
Expand Down
32 changes: 28 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/Binder_Invocation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ private BoundExpression BindInvocationExpression(
return result;
}

#nullable enable
private BoundExpression BindDynamicInvocation(
SyntaxNode node,
BoundExpression expression,
Expand All @@ -391,10 +392,11 @@ private BoundExpression BindDynamicInvocation(
CheckNamedArgumentsForDynamicInvocation(arguments, diagnostics);

bool hasErrors = false;
BoundExpression? receiver;
if (expression.Kind == BoundKind.MethodGroup)
{
BoundMethodGroup methodGroup = (BoundMethodGroup)expression;
BoundExpression receiver = methodGroup.ReceiverOpt;
receiver = methodGroup.ReceiverOpt;

// receiver is null if we are calling a static method declared on an outer class via its simple name:
if (receiver != null)
Expand All @@ -414,6 +416,7 @@ private BoundExpression BindDynamicInvocation(
// the runtime binder would ignore the receiver, but in a ctor initializer we can't read "this" before
// the base constructor is called. We need to handle this as a type qualified static method call.
// Also applicable to things like field initializers, which run before the ctor initializer.
Debug.Assert(ContainingType is not null);
expression = methodGroup.Update(
methodGroup.TypeArgumentsOpt,
methodGroup.Name,
Expand Down Expand Up @@ -456,12 +459,21 @@ private BoundExpression BindDynamicInvocation(
else
{
expression = BindToNaturalType(expression, diagnostics);

if (expression is BoundDynamicMemberAccess memberAccess)
{
receiver = memberAccess.Receiver;
}
else
{
receiver = expression;
}
}

ImmutableArray<BoundExpression> argArray = BuildArgumentsForDynamicInvocation(arguments, diagnostics);
var refKindsArray = arguments.RefKinds.ToImmutableOrNull();

hasErrors &= ReportBadDynamicArguments(node, argArray, refKindsArray, diagnostics, queryClause);
hasErrors &= ReportBadDynamicArguments(node, receiver, argArray, refKindsArray, diagnostics, queryClause);

return new BoundDynamicInvocation(
node,
Expand All @@ -473,6 +485,7 @@ private BoundExpression BindDynamicInvocation(
type: Compilation.DynamicType,
hasErrors: hasErrors);
}
#nullable disable

private void CheckNamedArgumentsForDynamicInvocation(AnalyzedArguments arguments, BindingDiagnosticBag diagnostics)
{
Expand Down Expand Up @@ -519,16 +532,26 @@ private ImmutableArray<BoundExpression> BuildArgumentsForDynamicInvocation(Analy
}

// Returns true if there were errors.
#nullable enable
private static bool ReportBadDynamicArguments(
SyntaxNode node,
BoundExpression? receiver,
ImmutableArray<BoundExpression> arguments,
ImmutableArray<RefKind> refKinds,
BindingDiagnosticBag diagnostics,
CSharpSyntaxNode queryClause)
CSharpSyntaxNode? queryClause)
{
bool hasErrors = false;
bool reportedBadQuery = false;

if (receiver != null && !IsLegalDynamicOperand(receiver))
{
// Cannot perform a dynamic invocation on an expression with type '{0}'.
Debug.Assert(receiver.Type is not null);
Error(diagnostics, ErrorCode.ERR_CannotDynamicInvokeOnExpression, receiver.Syntax, receiver.Type);
hasErrors = true;
}

if (!refKinds.IsDefault)
{
for (int argIndex = 0; argIndex < refKinds.Length; argIndex++)
Expand Down Expand Up @@ -576,7 +599,7 @@ private static bool ReportBadDynamicArguments(
{
// Lambdas,anonymous methods and method groups are the typeless expressions that
// are not usable as dynamic arguments; if we get here then the expression must have a type.
Debug.Assert((object)arg.Type != null);
Debug.Assert((object?)arg.Type != null);
// error CS1978: Cannot use an expression of type 'int*' as an argument to a dynamically dispatched operation

Error(diagnostics, ErrorCode.ERR_BadDynamicMethodArg, arg.Syntax, arg.Type);
Expand All @@ -586,6 +609,7 @@ private static bool ReportBadDynamicArguments(
}
return hasErrors;
}
#nullable disable

private BoundExpression BindDelegateInvocation(
SyntaxNode node,
Expand Down
5 changes: 4 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -7905,4 +7905,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_NoModifiersOnUsing" xml:space="preserve">
<value>Modifiers cannot be placed on using declarations</value>
</data>
</root>
<data name="ERR_CannotDynamicInvokeOnExpression" xml:space="preserve">
<value>Cannot perform a dynamic invocation on an expression with type '{0}'.</value>
</data>
</root>
5 changes: 5 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2301,10 +2301,15 @@ internal enum ErrorCode
ERR_ParamsCollectionMissingConstructor = 9228,

ERR_NoModifiersOnUsing = 9229,
ERR_CannotDynamicInvokeOnExpression = 9230,

#endregion

// Note: you will need to do the following after adding errors:
// 1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)

// Note: you will need to do the following after adding warnings:
// 1) Re-generate compiler code (eng\generate-compiler-code.cmd).
// 2) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)
}
}
1 change: 1 addition & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2431,6 +2431,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
case ErrorCode.ERR_ParamsCollectionExtensionAddMethod:
case ErrorCode.ERR_ParamsCollectionMissingConstructor:
case ErrorCode.ERR_NoModifiersOnUsing:
case ErrorCode.ERR_CannotDynamicInvokeOnExpression:
return false;
default:
// NOTE: All error codes must be explicitly handled in this switch statement
Expand Down
7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 7f05bbb

Please sign in to comment.