Skip to content

Commit

Permalink
Address feedback from previous PRs
Browse files Browse the repository at this point in the history
  • Loading branch information
@AlekseyTs
AlekseyTs committed Jan 8, 2023
1 parent a102b6c commit 808952a
Show file tree
Hide file tree
Showing 5 changed files with 248 additions and 98 deletions.
8 changes: 6 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/InMethodBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,12 @@ internal sealed class InMethodBinder : LocalScopeBinder

#if DEBUG
/// <summary>
/// This map is used to by MethodCompiler.BindMethodBody and Binder.BindIdentifier
/// to validate some assumptions around identifiers.
/// This map is used by <see cref="MethodCompiler.BindMethodBody(MethodSymbol, TypeCompilationState, BindingDiagnosticBag, bool, BoundNode?, bool, out ImportChain?, out bool, out bool, out MethodBodySemanticModel.InitialState)"/>
/// and <see cref="Binder.BindIdentifier"/> to validate some assumptions around identifiers.
///
/// Values in the dictionary are bit flags.
/// 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;
#endif
Expand Down
4 changes: 0 additions & 4 deletions src/Compilers/CSharp/Portable/Binder/WithPrimaryConstructorParametersBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +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.

// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Collections.Generic;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Symbols;
Expand Down
5 changes: 2 additions & 3 deletions src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2000,7 +2000,7 @@ syntaxNode is ConstructorDeclarationSyntax constructorSyntax &&
}

#if DEBUG
// Predict all identifiers in the body that should go though Binder.BindIdentifier method
// Predict all identifiers in the body that should go through Binder.BindIdentifier method
static void buildIdentifierMapOfBindIdentifierTargets(
CSharpSyntaxNode syntaxNode, Binder bodyBinder,
out InMethodBinder? inMethodBinder, out ConcurrentDictionary<IdentifierNameSyntax, int>? identifierMap)
Expand All @@ -2017,7 +2017,7 @@ static void buildIdentifierMapOfBindIdentifierTargets(
Binder current = bodyBinder;
while (current is not InMethodBinder)
{
current = current!.Next!;
current = current.Next!;
}

inMethodBinder = (InMethodBinder)current;
Expand Down Expand Up @@ -2147,7 +2147,6 @@ static void assertBindIdentifierTargets(InMethodBinder? inMethodBinder, Concurre
{
if (flags != (1 | 2))
{

if (id.IsMissing)
{
continue;
Expand Down
96 changes: 38 additions & 58 deletions src/Compilers/CSharp/Portable/Symbols/Synthesized/Records/SynthesizedRecordConstructor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ bool checkParameterReferencesInNode(CSharpSyntaxNode? node, Binder binder)
return true;
}

var nodesOfInterest = node.DescendantNodesAndSelf(descendIntoChildren: childrenNeedChecking, descendIntoTrivia: false).Where(nodeNeedsChecking);
var nodesOfInterest = node.DescendantNodesAndSelf(descendIntoChildren: childrenNeedChecking, descendIntoTrivia: false);

foreach (var n in nodesOfInterest)
{
Expand All @@ -260,8 +260,43 @@ bool checkParameterReferencesInNode(CSharpSyntaxNode? node, Binder binder)

case IdentifierNameSyntax id:

string name = id.Identifier.ValueText;
Debug.Assert(namesToCheck.Contains(name));
if (!namesToCheck.Contains(id.Identifier.ValueText))
{
continue;
}

switch (id.Parent)
{
case MemberAccessExpressionSyntax memberAccess:
if (memberAccess.Expression != id)
{
continue;
}
break;

case QualifiedNameSyntax qualifiedName:
if (qualifiedName.Left != id)
{
continue;
}
break;

case AssignmentExpressionSyntax assignment:
if (assignment.Left == id &&
assignment.Parent?.Kind() is SyntaxKind.ObjectInitializerExpression or SyntaxKind.WithInitializerExpression)
{
continue;
}
break;
}

if (SyntaxFacts.IsInTypeOnlyContext(id) &&
!(id.Parent is BinaryExpressionSyntax { RawKind: (int)SyntaxKind.IsExpression } isExpression &&
isExpression.Right == id))
{
continue;
}

if (!checkIdentifier(enclosingBinder, id))
{
return false;
Expand All @@ -276,9 +311,6 @@ bool checkParameterReferencesInNode(CSharpSyntaxNode? node, Binder binder)
}

break;

default:
throw ExceptionUtilities.UnexpectedValue(n.Kind());
}
}

Expand Down Expand Up @@ -737,58 +769,6 @@ static bool childrenNeedChecking(SyntaxNode n)

return true;
}

bool nodeNeedsChecking(SyntaxNode n)
{
switch (n)
{
case AnonymousFunctionExpressionSyntax:
case QueryExpressionSyntax:
return true;

case IdentifierNameSyntax id:

switch (id.Parent)
{
case MemberAccessExpressionSyntax memberAccess:
if (memberAccess.Expression != id)
{
return false;
}
break;

case QualifiedNameSyntax qualifiedName:
if (qualifiedName.Left != id)
{
return false;
}
break;

case AssignmentExpressionSyntax assignment:
if (assignment.Left == id &&
assignment.Parent?.Kind() is SyntaxKind.ObjectInitializerExpression or SyntaxKind.WithInitializerExpression)
{
return false;
}
break;
}

if (SyntaxFacts.IsInTypeOnlyContext(id) &&
!(id.Parent is BinaryExpressionSyntax { RawKind: (int)SyntaxKind.IsExpression } isExpression &&
isExpression.Right == id))
{
return false;
}

if (namesToCheck.Contains(id.Identifier.ValueText))
{
return true;
}
break;
}

return false;
}
}
}
}
Loading

0 comments on commit 808952a

Please sign in to comment.