Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disallow dynamic calls on ref struct receivers #72674

Merged
merged 8 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)
333fred marked this conversation as resolved.
Show resolved Hide resolved

// 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.

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
Loading