Skip to content

Commit cabcb95

Browse files
RikkiGibsonRikkiGibson
authored
Use helpers in param-nullchecking emit (#57615)
1 parent 38e86e0 commit cabcb95

File tree

10 files changed

+887
-436
lines changed

10 files changed

+887
-436
lines changed

‎src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,18 +1289,19 @@ internal Cci.IMethodReference Translate(
12891289
}
12901290
}
12911291

1292+
#nullable enable
12921293
private Cci.IMethodReference Translate(
12931294
MethodSymbol methodSymbol,
12941295
SyntaxNode syntaxNodeOpt,
12951296
DiagnosticBag diagnostics,
12961297
bool needDeclaration)
12971298
{
1298-
object reference;
1299+
object? reference;
12991300
Cci.IMethodReference methodRef;
13001301
NamedTypeSymbol container = methodSymbol.ContainingType;
13011302

13021303
// Method of anonymous type being translated
1303-
if (container.IsAnonymousType)
1304+
if (container?.IsAnonymousType == true)
13041305
{
13051306
Debug.Assert(!needDeclaration);
13061307
methodSymbol = AnonymousTypeManager.TranslateAnonymousTypeMethodSymbol(methodSymbol);
@@ -1363,6 +1364,7 @@ private Cci.IMethodReference Translate(
13631364

13641365
return methodSymbol.GetCciAdapter();
13651366
}
1367+
#nullable disable
13661368

13671369
internal Cci.IMethodReference TranslateOverriddenMethodReference(
13681370
MethodSymbol methodSymbol,
@@ -1777,6 +1779,39 @@ internal void EnsureNativeIntegerAttributeExists()
17771779
EnsureEmbeddableAttributeExists(EmbeddableAttributes.NativeIntegerAttribute);
17781780
}
17791781

1782+
#nullable enable
1783+
/// <summary>
1784+
/// Creates the ThrowIfNull and Throw helpers if needed.
1785+
/// </summary>
1786+
/// <remarks>
1787+
/// The ThrowIfNull and Throw helpers are modeled off of the helpers on ArgumentNullException.
1788+
/// https://github.com/dotnet/runtime/blob/22663769611ba89cd92d14cfcb76e287f8af2335/src/libraries/System.Private.CoreLib/src/System/ArgumentNullException.cs#L56-L69
1789+
/// </remarks>
1790+
internal MethodSymbol EnsureThrowIfNullFunctionExists(SyntaxNode syntaxNode, SyntheticBoundNodeFactory factory, DiagnosticBag? diagnostics)
1791+
{
1792+
var privateImplClass = GetPrivateImplClass(syntaxNode, diagnostics);
1793+
var throwIfNullAdapter = privateImplClass.GetMethod(PrivateImplementationDetails.SynthesizedThrowIfNullFunctionName);
1794+
if (throwIfNullAdapter is null)
1795+
{
1796+
TypeSymbol returnType = factory.SpecialType(SpecialType.System_Void);
1797+
TypeSymbol argumentType = factory.SpecialType(SpecialType.System_Object);
1798+
TypeSymbol paramNameType = factory.SpecialType(SpecialType.System_String);
1799+
var sourceModule = SourceModule;
1800+
1801+
// use add-then-get pattern to ensure the symbol exists, and then ensure we use the single "canonical" instance added by whichever thread won the race.
1802+
privateImplClass.TryAddSynthesizedMethod(new SynthesizedThrowMethod(sourceModule, privateImplClass, returnType, paramNameType).GetCciAdapter());
1803+
var actuallyAddedThrowMethod = (SynthesizedThrowMethod)privateImplClass.GetMethod(PrivateImplementationDetails.SynthesizedThrowFunctionName)!.GetInternalSymbol()!;
1804+
privateImplClass.TryAddSynthesizedMethod(new SynthesizedThrowIfNullMethod(sourceModule, privateImplClass, actuallyAddedThrowMethod, returnType, argumentType, paramNameType).GetCciAdapter());
1805+
throwIfNullAdapter = privateImplClass.GetMethod(PrivateImplementationDetails.SynthesizedThrowIfNullFunctionName)!;
1806+
}
1807+
1808+
var internalSymbol = (SynthesizedThrowIfNullMethod)throwIfNullAdapter.GetInternalSymbol()!;
1809+
// the ThrowMethod referenced by the ThrowIfNullMethod must be the same instance as the ThrowMethod contained in the PrivateImplementationDetails
1810+
Debug.Assert((object?)privateImplClass.GetMethod(PrivateImplementationDetails.SynthesizedThrowFunctionName)!.GetInternalSymbol() == internalSymbol.ThrowMethod);
1811+
return internalSymbol;
1812+
}
1813+
#nullable disable
1814+
17801815
public override IEnumerable<Cci.INamespaceTypeDefinition> GetAdditionalTopLevelTypeDefinitions(EmitContext context)
17811816
{
17821817
return GetAdditionalTopLevelTypes()

‎src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_NullChecking.cs‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,17 @@ internal static ImmutableArray<BoundStatement> TryConstructNullCheckedStatementL
3232
SyntheticBoundNodeFactory factory)
3333
{
3434
ArrayBuilder<BoundStatement>? statementList = null;
35+
MethodSymbol? throwIfNullMethod = null;
3536
foreach (ParameterSymbol param in parameters)
3637
{
3738
if (param.IsNullChecked)
3839
{
39-
Debug.Assert(!param.Type.IsValueType || param.Type.IsNullableTypeOrTypeParameter());
40+
var isNullCheckableValueType = param.Type.IsNullableTypeOrTypeParameter() || param.Type.IsPointerOrFunctionPointer();
41+
Debug.Assert(!param.Type.IsValueType || isNullCheckableValueType);
4042
statementList ??= ArrayBuilder<BoundStatement>.GetInstance();
41-
var constructedIf = ConstructIfStatementForParameter(param, factory);
43+
var constructedIf = isNullCheckableValueType
44+
? ConstructDirectNullCheck(param, factory)
45+
: ConstructNullCheckHelperCall(param, ref throwIfNullMethod, factory);
4246
statementList.Add(constructedIf);
4347
}
4448
}
@@ -49,10 +53,28 @@ internal static ImmutableArray<BoundStatement> TryConstructNullCheckedStatementL
4953

5054
statementList.AddRange(existingStatements);
5155
return statementList.ToImmutableAndFree();
56+
}
57+
58+
private static BoundStatement ConstructNullCheckHelperCall(ParameterSymbol parameter, ref MethodSymbol? throwIfNullMethod, SyntheticBoundNodeFactory factory)
59+
{
60+
if (throwIfNullMethod is null)
61+
{
62+
var module = factory.ModuleBuilderOpt!;
63+
var diagnosticSyntax = factory.CurrentFunction.GetNonNullSyntaxNode();
64+
var diagnostics = factory.Diagnostics.DiagnosticBag;
65+
Debug.Assert(diagnostics is not null);
66+
throwIfNullMethod = module.EnsureThrowIfNullFunctionExists(diagnosticSyntax, factory, diagnostics);
67+
}
5268

69+
var call = factory.Call(
70+
receiver: null,
71+
throwIfNullMethod,
72+
arg0: factory.Convert(factory.SpecialType(SpecialType.System_Object), factory.Parameter(parameter)),
73+
arg1: factory.StringLiteral(parameter.Name));
74+
return factory.HiddenSequencePoint(factory.ExpressionStatement(call));
5375
}
5476

55-
private static BoundStatement ConstructIfStatementForParameter(ParameterSymbol parameter, SyntheticBoundNodeFactory factory)
77+
private static BoundStatement ConstructDirectNullCheck(ParameterSymbol parameter, SyntheticBoundNodeFactory factory)
5678
{
5779
BoundExpression paramIsNullCondition;
5880
var loweredLeft = factory.Parameter(parameter);
@@ -63,6 +85,13 @@ private static BoundStatement ConstructIfStatementForParameter(ParameterSymbol p
6385
}
6486
else
6587
{
88+
// Examples of how we might get here:
89+
// int*
90+
// delegate*<...>
91+
// T where T : int? (via some indirection)
92+
Debug.Assert(parameter.Type.IsPointerOrFunctionPointer()
93+
|| (parameter.Type.IsNullableTypeOrTypeParameter() && !parameter.Type.IsNullableType()));
94+
6695
paramIsNullCondition = factory.MakeNullCheck(loweredLeft.Syntax, loweredLeft, BinaryOperatorKind.Equal);
6796
}
6897

‎src/Compilers/CSharp/Portable/Lowering/MethodToClassRewriter.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,12 @@ public override BoundNode VisitLoweredConditionalAccess(BoundLoweredConditionalA
577577
return null;
578578
}
579579

580-
if (method.ContainingType.IsAnonymousType)
580+
if (method.ContainingType is null)
581+
{
582+
Debug.Assert(method is SynthesizedGlobalMethodSymbol);
583+
return method;
584+
}
585+
else if (method.ContainingType.IsAnonymousType)
581586
{
582587
// Method of an anonymous type
583588
var newType = (NamedTypeSymbol)TypeMap.SubstituteType(method.ContainingType).AsTypeSymbolOnly();

‎src/Compilers/CSharp/Portable/Lowering/SyntheticBoundNodeFactory.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1541,7 +1541,7 @@ internal BoundExpression MakeNullCheck(SyntaxNode syntax, BoundExpression rewrit
15411541
(object)exprType == null ||
15421542
exprType.IsNullableTypeOrTypeParameter() ||
15431543
!exprType.IsValueType ||
1544-
exprType.IsPointerType());
1544+
exprType.IsPointerOrFunctionPointer());
15451545

15461546
TypeSymbol boolType = Compilation.GetSpecialType(CodeAnalysis.SpecialType.System_Boolean);
15471547

‎src/Compilers/CSharp/Portable/Symbols/MemberSymbolExtensions.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,7 @@ internal static bool IsParameterlessConstructor(this MethodSymbol method)
440440
return method.MethodKind == MethodKind.Constructor && method.ParameterCount == 0;
441441
}
442442

443+
#nullable enable
443444
/// <summary>
444445
/// Returns true if the method is the default constructor synthesized for struct types, and
445446
/// if <paramref name="requireZeroInit"/> is true, the constructor simply zero-inits the instance.
@@ -453,7 +454,7 @@ internal static bool IsParameterlessConstructor(this MethodSymbol method)
453454
internal static bool IsDefaultValueTypeConstructor(this MethodSymbol method, bool requireZeroInit)
454455
{
455456
if (method.IsImplicitlyDeclared &&
456-
method.ContainingType.IsValueType &&
457+
method.ContainingType?.IsValueType == true &&
457458
method.IsParameterlessConstructor())
458459
{
459460
if (!requireZeroInit)
@@ -469,6 +470,7 @@ internal static bool IsDefaultValueTypeConstructor(this MethodSymbol method, boo
469470
}
470471
return false;
471472
}
473+
#nullable disable
472474

473475
/// <summary>
474476
/// Indicates whether the method should be emitted.

‎src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -802,7 +802,7 @@ internal static void ReportParameterNullCheckingErrors(DiagnosticBag diagnostics
802802
{
803803
diagnostics.Add(useSiteInfo.DiagnosticInfo, location);
804804
}
805-
if (parameter.Type.IsValueType)
805+
if (parameter.Type.IsValueType && !parameter.Type.IsPointerOrFunctionPointer())
806806
{
807807
if (!parameter.Type.IsNullableTypeOrTypeParameter())
808808
{

‎src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedThrowIfNullMethod.cs‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using Microsoft.CodeAnalysis.CodeGen;
7+
8+
namespace Microsoft.CodeAnalysis.CSharp.Symbols
9+
{
10+
/// <summary>
11+
/// Throws a System.ArgumentNullException if 'argument' is null.
12+
/// </summary>
13+
internal sealed class SynthesizedThrowIfNullMethod : SynthesizedGlobalMethodSymbol
14+
{
15+
internal MethodSymbol ThrowMethod { get; }
16+
internal SynthesizedThrowIfNullMethod(SourceModuleSymbol containingModule, PrivateImplementationDetails privateImplType, MethodSymbol throwMethod, TypeSymbol returnType, TypeSymbol argumentParamType, TypeSymbol paramNameParamType)
17+
: base(containingModule, privateImplType, returnType, PrivateImplementationDetails.SynthesizedThrowIfNullFunctionName)
18+
{
19+
ThrowMethod = throwMethod;
20+
21+
this.SetParameters(ImmutableArray.Create<ParameterSymbol>(
22+
SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(argumentParamType), ordinal: 0, RefKind.None, "argument"),
23+
SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(paramNameParamType), ordinal: 1, RefKind.None, "paramName")));
24+
}
25+
26+
internal override void GenerateMethodBody(TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
27+
{
28+
SyntheticBoundNodeFactory F = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics);
29+
F.CurrentFunction = this;
30+
31+
try
32+
{
33+
ParameterSymbol argument = this.Parameters[0];
34+
ParameterSymbol paramName = this.Parameters[1];
35+
36+
//if (argument is null)
37+
//{
38+
// Throw(paramName);
39+
//}
40+
41+
var body = F.Block(
42+
ImmutableArray<LocalSymbol>.Empty,
43+
F.If(
44+
F.Binary(BinaryOperatorKind.ObjectEqual, F.SpecialType(SpecialType.System_Boolean),
45+
F.Parameter(argument),
46+
F.Null(argument.Type)),
47+
F.ExpressionStatement(F.Call(receiver: null, ThrowMethod, F.Parameter(paramName)))),
48+
F.Return());
49+
50+
// NOTE: we created this block in its most-lowered form, so analysis is unnecessary
51+
F.CloseMethod(body);
52+
}
53+
catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
54+
{
55+
diagnostics.Add(ex.Diagnostic);
56+
F.CloseMethod(F.ThrowNull());
57+
}
58+
}
59+
}
60+
}

‎src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedThrowMethod.cs‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System.Collections.Immutable;
6+
using Microsoft.CodeAnalysis.CodeGen;
7+
8+
namespace Microsoft.CodeAnalysis.CSharp.Symbols
9+
{
10+
/// <summary>
11+
/// Throws a System.ArgumentNullException with the given 'paramName'.
12+
/// </summary>
13+
internal sealed class SynthesizedThrowMethod : SynthesizedGlobalMethodSymbol
14+
{
15+
internal SynthesizedThrowMethod(SourceModuleSymbol containingModule, PrivateImplementationDetails privateImplType, TypeSymbol returnType, TypeSymbol paramType)
16+
: base(containingModule, privateImplType, returnType, PrivateImplementationDetails.SynthesizedThrowFunctionName)
17+
{
18+
this.SetParameters(ImmutableArray.Create<ParameterSymbol>(SynthesizedParameterSymbol.Create(this, TypeWithAnnotations.Create(paramType), 0, RefKind.None, "paramName")));
19+
}
20+
21+
internal override void GenerateMethodBody(TypeCompilationState compilationState, BindingDiagnosticBag diagnostics)
22+
{
23+
SyntheticBoundNodeFactory F = new SyntheticBoundNodeFactory(this, this.GetNonNullSyntaxNode(), compilationState, diagnostics);
24+
F.CurrentFunction = this;
25+
26+
try
27+
{
28+
ParameterSymbol paramName = this.Parameters[0];
29+
30+
//throw new ArgumentNullException(paramName);
31+
32+
var body = F.Block(
33+
ImmutableArray<LocalSymbol>.Empty,
34+
F.Throw(F.New(F.WellKnownMethod(WellKnownMember.System_ArgumentNullException__ctorString), ImmutableArray.Create<BoundExpression>(F.Parameter(paramName)))));
35+
36+
// NOTE: we created this block in its most-lowered form, so analysis is unnecessary
37+
F.CloseMethod(body);
38+
}
39+
catch (SyntheticBoundNodeFactory.MissingPredefinedMember ex)
40+
{
41+
diagnostics.Add(ex.Diagnostic);
42+
F.CloseMethod(F.ThrowNull());
43+
}
44+
}
45+
}
46+
}

0 commit comments

Comments
 (0)