Skip to content

Commit

Permalink
Fix circularity issue with binding const
Browse files Browse the repository at this point in the history
  • Loading branch information
jcouv committed Oct 3, 2024
1 parent 244b7e5 commit dd9a999
Show file tree
Hide file tree
Showing 19 changed files with 167 additions and 40 deletions.
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -475,12 +475,12 @@ internal virtual ConsList<FieldSymbol> FieldsBeingBound
}
}

internal virtual LocalSymbol? LocalInProgress
internal virtual ConsList<LocalSymbol> LocalsInProgress
{
get
{
RoslynDebug.Assert(Next is object);
return this.Next.LocalInProgress;
return this.Next.LocalsInProgress;
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs
Original file line number Diff line number Diff line change
Expand Up @@ -867,10 +867,10 @@ private BoundExpression BindDeconstructionVariable(
CSharpSyntaxNode syntax,
BindingDiagnosticBag diagnostics)
{
SourceLocalSymbol localSymbol = LookupLocal(designation.Identifier);
SourceLocalSymbol? localSymbol = LookupLocal(designation.Identifier);

// is this a local?
if ((object)localSymbol != null)
if ((object?)localSymbol != null)
{
if (designation.Parent is DeclarationExpressionSyntax declExpr && declExpr.Designation == designation)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2065,7 +2065,7 @@ private BoundExpression BindNonMethod(SimpleNameSyntax node, Symbol symbol, Bind
}

var constantValueOpt = localSymbol.IsConst && !IsInsideNameof && !type.IsErrorType()
? localSymbol.GetConstantValue(node, this.LocalInProgress, diagnostics) : null;
? localSymbol.GetConstantValue(node, this.LocalsInProgress, diagnostics) : null;
return new BoundLocal(node, localSymbol, BoundLocalDeclarationKind.None, constantValueOpt: constantValueOpt, isNullableUnknown: isNullableUnknown, type: type, hasErrors: isError);
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/Binder_Patterns.cs
Original file line number Diff line number Diff line change
Expand Up @@ -876,7 +876,7 @@ private void BindPatternDesignation(
{
case SingleVariableDesignationSyntax singleVariableDesignation:
SyntaxToken identifier = singleVariableDesignation.Identifier;
SourceLocalSymbol localSymbol = this.LookupLocal(identifier);
SourceLocalSymbol? localSymbol = this.LookupLocal(identifier);

if (!permitDesignations && !identifier.IsMissing)
diagnostics.Add(ErrorCode.ERR_DesignatorBeneathPatternCombinator, identifier.GetLocation());
Expand Down
9 changes: 6 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs
Original file line number Diff line number Diff line change
Expand Up @@ -956,8 +956,8 @@ protected BoundLocalDeclaration BindVariableDeclaration(
CSharpSyntaxNode associatedSyntaxNode = null)
{
Debug.Assert(declarator != null);

return BindVariableDeclaration(LocateDeclaredVariableSymbol(declarator, typeSyntax, kind),
var binder = this.GetRequiredBinder(declarator);
return binder.BindVariableDeclaration(LocateDeclaredVariableSymbol(declarator, typeSyntax, kind),
kind,
isVar,
declarator,
Expand Down Expand Up @@ -1845,10 +1845,13 @@ private BoundExpression BindPossibleArrayInitializer(
return CheckValue(result, valueKind, diagnostics);
}

protected virtual SourceLocalSymbol LookupLocal(SyntaxToken nameToken)
#nullable enable
protected virtual SourceLocalSymbol? LookupLocal(SyntaxToken nameToken)
{
Debug.Assert(Next is not null);
return Next.LookupLocal(nameToken);
}
#nullable disable

protected virtual LocalFunctionSymbol LookupLocalFunction(SyntaxToken nameToken)
{
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ internal override ConsList<FieldSymbol> FieldsBeingBound
}
}

internal override LocalSymbol? LocalInProgress
internal override ConsList<LocalSymbol> LocalsInProgress
{
get
{
return null;
return ConsList<LocalSymbol>.Empty;
}
}

Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,14 @@ public override void VisitLocalDeclarationStatement(LocalDeclarationStatementSyn

public override void VisitVariableDeclarator(VariableDeclaratorSyntax node)
{
var oldEnclosing = _enclosing;
var localInProgressBinder = new LocalInProgressBinder(node.Identifier, _enclosing);
AddToMap(node, localInProgressBinder);

_enclosing = localInProgressBinder;
Visit(node.ArgumentList);
Visit(node.Initializer?.Value);
_enclosing = oldEnclosing;
}

public override void VisitReturnStatement(ReturnStatementSyntax node)
Expand Down
21 changes: 12 additions & 9 deletions src/Compilers/CSharp/Portable/Binder/LocalInProgressBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,8 @@
// 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 Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
{
Expand All @@ -17,19 +14,25 @@ namespace Microsoft.CodeAnalysis.CSharp
/// </summary>
internal sealed class LocalInProgressBinder : Binder
{
private readonly LocalSymbol _inProgress;
private readonly SyntaxToken _inProgressNameToken;

internal LocalInProgressBinder(LocalSymbol inProgress, Binder next)
internal LocalInProgressBinder(SyntaxToken nameToken, Binder next)
: base(next)
{
_inProgress = inProgress;
_inProgressNameToken = nameToken;
}

internal override LocalSymbol LocalInProgress
internal override ConsList<LocalSymbol> LocalsInProgress
{
get
{
return _inProgress;
var result = base.LocalsInProgress;
if (this.LookupLocal(_inProgressNameToken) is { } local)
{
return result.Push(local);
}

return result;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/CodeGen/Optimizer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2403,7 +2403,7 @@ public override ImmutableArray<SyntaxReference> DeclaringSyntaxReferences
get { throw new NotImplementedException(); }
}

internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics)
internal override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol> inProgress, BindingDiagnosticBag diagnostics)
{
throw new NotImplementedException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public override bool Equals(Symbol obj, TypeCompareKind compareKind)
internal override bool IsKnownToReferToTempIfReferenceType => false;
public override RefKind RefKind => RefKind.None;
internal override SynthesizedLocalKind SynthesizedKind => throw ExceptionUtilities.Unreachable();
internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics = null) => null;
internal override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol> inProgress, BindingDiagnosticBag diagnostics = null) => null;
internal override ReadOnlyBindingDiagnostic<AssemblySymbol> GetConstantValueDiagnostics(BoundExpression boundInitValue) => ReadOnlyBindingDiagnostic<AssemblySymbol>.Empty;
internal override SyntaxNode GetDeclaratorSyntax() => throw ExceptionUtilities.Unreachable();
internal override bool HasSourceLocation => false;
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/LocalSymbol.cs
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,9 @@ internal abstract bool IsCompilerGenerated
get;
}

internal abstract ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics = null);
#nullable enable
internal abstract ConstantValue? GetConstantValue(SyntaxNode node, ConsList<LocalSymbol>? inProgress, BindingDiagnosticBag? diagnostics = null);
#nullable disable

internal abstract ReadOnlyBindingDiagnostic<AssemblySymbol> GetConstantValueDiagnostics(BoundExpression boundInitValue);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ internal override bool IsCompilerGenerated
get { return false; }
}

internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics)
internal override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol> inProgress, BindingDiagnosticBag diagnostics)
{
return null;
}
Expand Down Expand Up @@ -547,34 +547,34 @@ protected override TypeWithAnnotations InferTypeOfVarVariable(BindingDiagnosticB

internal override SyntaxNode ForbiddenZone => _initializer;

#nullable enable
/// <summary>
/// Determine the constant value of this local and the corresponding diagnostics.
/// Set both to constantTuple in a single operation for thread safety.
/// </summary>
/// <param name="inProgress">Null for the initial call, non-null if we are in the process of evaluating a constant.</param>
/// <param name="boundInitValue">If we already have the bound node for the initial value, pass it in to avoid recomputing it.</param>
private void MakeConstantTuple(LocalSymbol inProgress, BoundExpression boundInitValue)
private void MakeConstantTuple(ConsList<LocalSymbol>? inProgress, BoundExpression? boundInitValue)
{
if (this.IsConst && _constantTuple == null)
{
var value = Microsoft.CodeAnalysis.ConstantValue.Bad;
var diagnostics = BindingDiagnosticBag.GetInstance();
Debug.Assert(inProgress != this);
Debug.Assert(inProgress?.Contains(this) != true);
var type = this.Type;
if (boundInitValue == null)
{
var inProgressBinder = new LocalInProgressBinder(this, this._initializerBinder);
boundInitValue = inProgressBinder.BindVariableOrAutoPropInitializerValue(_initializer, this.RefKind, type, diagnostics);
boundInitValue = this._initializerBinder.BindVariableOrAutoPropInitializerValue(_initializer, this.RefKind, type, diagnostics);
}

value = ConstantValueUtils.GetAndValidateConstantValue(boundInitValue, this, type, _initializer.Value, diagnostics);
Interlocked.CompareExchange(ref _constantTuple, new EvaluatedConstant(value, diagnostics.ToReadOnlyAndFree()), null);
}
}

internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics = null)
internal override ConstantValue? GetConstantValue(SyntaxNode node, ConsList<LocalSymbol>? inProgress, BindingDiagnosticBag? diagnostics = null)
{
if (this.IsConst && inProgress == this)
if (this.IsConst && inProgress?.Contains(this) == true)
{
if (diagnostics != null)
{
Expand All @@ -594,6 +594,7 @@ internal override ReadOnlyBindingDiagnostic<AssemblySymbol> GetConstantValueDiag
MakeConstantTuple(inProgress: null, boundInitValue: boundInitValue);
return _constantTuple == null ? ReadOnlyBindingDiagnostic<AssemblySymbol>.Empty : _constantTuple.Diagnostics;
}
#nullable disable
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using System.Diagnostics;
using System.Runtime.CompilerServices;
using System.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
Expand Down Expand Up @@ -187,7 +188,7 @@ internal sealed override bool IsCompilerGenerated

internal sealed override ScopedKind Scope => ScopedKind.None;

internal sealed override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics)
internal sealed override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol> inProgress, BindingDiagnosticBag diagnostics)
{
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public override RefKind RefKind
/// </summary>
internal override ScopedKind Scope => throw new System.NotImplementedException();

internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag diagnostics)
internal override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol> inProgress, BindingDiagnosticBag diagnostics)
{
return _originalVariable.GetConstantValue(node, inProgress, diagnostics);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ public override bool Equals(Symbol other, TypeCompareKind compareKind)
internal override bool IsKnownToReferToTempIfReferenceType => _underlyingLocal.IsKnownToReferToTempIfReferenceType;
internal override bool IsCompilerGenerated => _underlyingLocal.IsCompilerGenerated;
internal override ScopedKind Scope => _underlyingLocal.Scope;
internal override ConstantValue GetConstantValue(SyntaxNode node, LocalSymbol inProgress, BindingDiagnosticBag? diagnostics = null) =>
internal override ConstantValue GetConstantValue(SyntaxNode node, ConsList<LocalSymbol>? inProgress, BindingDiagnosticBag? diagnostics = null) =>
_underlyingLocal.GetConstantValue(node, inProgress, diagnostics);
internal override ReadOnlyBindingDiagnostic<AssemblySymbol> GetConstantValueDiagnostics(BoundExpression boundInitValue) =>
_underlyingLocal.GetConstantValueDiagnostics(boundInitValue);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ internal static bool CanHaveAssociatedLocalBinder(this SyntaxNode syntax)
case SyntaxKind.PrimaryConstructorBaseType:
case SyntaxKind.CheckedExpression:
case SyntaxKind.UncheckedExpression:
case SyntaxKind.VariableDeclarator:
return true;

case SyntaxKind.RecordStructDeclaration:
Expand Down
Loading

0 comments on commit dd9a999

Please sign in to comment.