Skip to content

Commit

Permalink
Nullable annotate InMethodyBinder
Browse files Browse the repository at this point in the history 
This is in response to dotnet#73022 where there is a null reference exception
during binder. The cause is, very likely, a `null` symbol being passed
into the binder. It's not possible to fully root cause this at the
moment but adding asserts and NRT annotations to help us track it down
in the future.
  • Loading branch information
@jaredpar
jaredpar committed Apr 29, 2024
1 parent 1abc045 commit 3fc2491
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 16 deletions.
7 changes: 6 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/BinderFactory.BinderFactoryVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ public override Binder VisitMethodDeclaration(MethodDeclarationSyntax methodDecl
if (usage == NodeUsage.MethodBody)
{
method = method ?? GetMethodSymbol(methodDecl, resultBinder);
Debug.Assert(method is not null);
resultBinder = new InMethodBinder(method, resultBinder);
}

Expand Down Expand Up @@ -247,6 +248,7 @@ public override Binder VisitDestructorDeclaration(DestructorDeclarationSyntax pa
resultBinder = VisitCore(parent.Parent);

SourceMemberMethodSymbol method = GetMethodSymbol(parent, resultBinder);
Debug.Assert(method is not null);
resultBinder = new InMethodBinder(method, resultBinder);

resultBinder = resultBinder.WithUnsafeRegionIfNecessary(parent.Modifiers);
Expand Down Expand Up @@ -490,8 +492,9 @@ private static string GetPropertyOrEventName(BasePropertyDeclarationSyntax baseP
}
}

#nullable enable
// Get the correct methods symbol within container that corresponds to the given method syntax.
private SourceMemberMethodSymbol GetMethodSymbol(BaseMethodDeclarationSyntax baseMethodDeclarationSyntax, Binder outerBinder)
private SourceMemberMethodSymbol? GetMethodSymbol(BaseMethodDeclarationSyntax baseMethodDeclarationSyntax, Binder outerBinder)
{
if (baseMethodDeclarationSyntax == _memberDeclarationOpt)
{
Expand All @@ -508,6 +511,8 @@ private SourceMemberMethodSymbol GetMethodSymbol(BaseMethodDeclarationSyntax bas
return (SourceMemberMethodSymbol)GetMemberSymbol(methodName, baseMethodDeclarationSyntax.FullSpan, container, SymbolKind.Method);
}

#nullable disable

private SourcePropertySymbol GetPropertySymbol(BasePropertyDeclarationSyntax basePropertyDeclarationSyntax, Binder outerBinder)
{
Debug.Assert(basePropertyDeclarationSyntax.Kind() == SyntaxKind.PropertyDeclaration || basePropertyDeclarationSyntax.Kind() == SyntaxKind.IndexerDeclaration);
Expand Down
28 changes: 13 additions & 15 deletions src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable disable

using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.Immutable;
Expand All @@ -21,9 +19,9 @@ namespace Microsoft.CodeAnalysis.CSharp
/// </summary>
internal sealed class InMethodBinder : LocalScopeBinder
{
private MultiDictionary<string, ParameterSymbol> _lazyParameterMap;
private MultiDictionary<string, ParameterSymbol>? _lazyParameterMap;
private readonly MethodSymbol _methodSymbol;
private SmallDictionary<string, Symbol> _lazyDefinitionMap;
private SmallDictionary<string, Symbol>? _lazyDefinitionMap;

#if DEBUG
/// <summary>
Expand All @@ -34,7 +32,7 @@ internal sealed class InMethodBinder : LocalScopeBinder
/// MethodCompiler.BindMethodBody adds keys with flag == 1 before binding a method body.
/// Binder.BindIdentifier adds or updates keys with flag == 2.
/// </summary>
public ConcurrentDictionary<IdentifierNameSyntax, int> IdentifierMap;
public ConcurrentDictionary<IdentifierNameSyntax, int>? IdentifierMap;
#endif

public InMethodBinder(MethodSymbol owner, Binder enclosing)
Expand All @@ -56,12 +54,12 @@ public InMethodBinder(MethodSymbol owner, Binder enclosing)
}
}

protected override SourceLocalSymbol LookupLocal(SyntaxToken nameToken)
protected override SourceLocalSymbol? LookupLocal(SyntaxToken nameToken)
{
return null;
}

protected override LocalFunctionSymbol LookupLocalFunction(SyntaxToken nameToken)
protected override LocalFunctionSymbol? LookupLocalFunction(SyntaxToken nameToken)
{
return null;
}
Expand Down Expand Up @@ -102,15 +100,15 @@ internal override bool IsIndirectlyInIterator
}
}

internal override GeneratedLabelSymbol BreakLabel
internal override GeneratedLabelSymbol? BreakLabel
{
get
{
return null;
}
}

internal override GeneratedLabelSymbol ContinueLabel
internal override GeneratedLabelSymbol? ContinueLabel
{
get
{
Expand Down Expand Up @@ -143,7 +141,7 @@ internal override TypeWithAnnotations GetIteratorElementType()
}

internal static TypeWithAnnotations GetIteratorElementTypeFromReturnType(CSharpCompilation compilation,
RefKind refKind, TypeSymbol returnType, Location errorLocation, BindingDiagnosticBag diagnostics)
RefKind refKind, TypeSymbol returnType, Location? errorLocation, BindingDiagnosticBag? diagnostics)
{
if (refKind == RefKind.None && returnType.Kind == SymbolKind.NamedType)
{
Expand Down Expand Up @@ -239,7 +237,7 @@ internal override void AddLookupSymbolsInfoInSingleBinder(LookupSymbolsInfo resu
}
}

private static bool ReportConflictWithParameter(Symbol parameter, Symbol newSymbol, string name, Location newLocation, BindingDiagnosticBag diagnostics)
private static bool ReportConflictWithParameter(Symbol parameter, Symbol? newSymbol, string name, Location newLocation, BindingDiagnosticBag diagnostics)
{
#if DEBUG
var locations = parameter.Locations;
Expand All @@ -251,7 +249,7 @@ private static bool ReportConflictWithParameter(Symbol parameter, Symbol newSymb
SymbolKind parameterKind = parameter.Kind;

// Quirk of the way we represent lambda parameters.
SymbolKind newSymbolKind = (object)newSymbol == null ? SymbolKind.Parameter : newSymbol.Kind;
SymbolKind newSymbolKind = newSymbol is null ? SymbolKind.Parameter : newSymbol.Kind;

if (newSymbolKind == SymbolKind.ErrorType)
{
Expand All @@ -269,7 +267,7 @@ private static bool ReportConflictWithParameter(Symbol parameter, Symbol newSymb
return true;

case SymbolKind.Method:
if (((MethodSymbol)newSymbol).MethodKind == MethodKind.LocalFunction)
if (((MethodSymbol)newSymbol!).MethodKind == MethodKind.LocalFunction)
{
goto case SymbolKind.Parameter;
}
Expand Down Expand Up @@ -297,7 +295,7 @@ private static bool ReportConflictWithParameter(Symbol parameter, Symbol newSymb
return true;

case SymbolKind.Method:
if (((MethodSymbol)newSymbol).MethodKind == MethodKind.LocalFunction)
if (((MethodSymbol)newSymbol!).MethodKind == MethodKind.LocalFunction)
{
goto case SymbolKind.Parameter;
}
Expand Down Expand Up @@ -340,7 +338,7 @@ internal override bool EnsureSingleDefinition(Symbol symbol, string name, Locati
_lazyDefinitionMap = map;
}

Symbol existingDeclaration;
Symbol? existingDeclaration;
if (map.TryGetValue(name, out existingDeclaration))
{
return ReportConflictWithParameter(existingDeclaration, symbol, name, location, diagnostics);
Expand Down

0 comments on commit 3fc2491

Please sign in to comment.