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

Fix circularity issue with binding const local #75371

Merged
merged 5 commits into from
Oct 11, 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
13 changes: 12 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,18 @@ public override void VisitLocalDeclarationStatement(LocalDeclarationStatementSyn
public override void VisitVariableDeclarator(VariableDeclaratorSyntax node)
{
Visit(node.ArgumentList);
Visit(node.Initializer?.Value);

if (node.Initializer is { } initializer)
{
var enclosing = _enclosing;
if (node.Parent is VariableDeclarationSyntax { Parent: LocalDeclarationStatementSyntax { IsConst: true } })
{
enclosing = new LocalInProgressBinder(initializer, _enclosing);
AddToMap(initializer, enclosing);
}

Visit(initializer.Value, enclosing);
}
}

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

namespace Microsoft.CodeAnalysis.CSharp
{
Expand All @@ -17,20 +18,28 @@ namespace Microsoft.CodeAnalysis.CSharp
/// </summary>
internal sealed class LocalInProgressBinder : Binder
{
private readonly LocalSymbol _inProgress;
public readonly EqualsValueClauseSyntax InitializerSyntax;
private LocalSymbol? _localSymbol;

internal LocalInProgressBinder(LocalSymbol inProgress, Binder next)
internal LocalInProgressBinder(EqualsValueClauseSyntax initializerSyntax, Binder next)
: base(next)
{
_inProgress = inProgress;
InitializerSyntax = initializerSyntax;
}

internal override LocalSymbol LocalInProgress
{
get
{
return _inProgress;
// The local symbol should have been initialized by now
Debug.Assert(_localSymbol is not null);
return _localSymbol;
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
RikkiGibson marked this conversation as resolved.
Show resolved Hide resolved
}
}

internal void SetLocalSymbol(LocalSymbol local)
{
Interlocked.CompareExchange(ref _localSymbol, local, null);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,25 @@ public LocalWithInitializer(

_initializer = initializer;
_initializerBinder = initializerBinder;
if (this.IsConst)
{
_initializerBinder = _initializerBinder.GetBinder(initializer) ?? new LocalInProgressBinder(_initializer, initializerBinder); // for error scenarios
recordConstInBinderChain();
}

void recordConstInBinderChain()
{
for (var binder = _initializerBinder; binder != null; binder = binder.Next)
{
if (binder is LocalInProgressBinder localInProgressBinder && localInProgressBinder.InitializerSyntax == _initializer)
{
localInProgressBinder.SetLocalSymbol(this);
return;
}
}

throw ExceptionUtilities.Unreachable();
}
}

protected override TypeWithAnnotations InferTypeOfVarVariable(BindingDiagnosticBag diagnostics)
Expand All @@ -563,8 +582,7 @@ private void MakeConstantTuple(LocalSymbol inProgress, BoundExpression boundInit
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);
Expand Down
181 changes: 178 additions & 3 deletions src/Compilers/CSharp/Test/Semantic/Semantics/ConstantTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3309,9 +3309,28 @@ public static void Main()
}
}";
CreateCompilation(source).VerifyDiagnostics(
// (6,51): error CS0133: The expression being assigned to 'b' must be constant
// const Func<int> a = () => { const int b = a(); return 1; };
Diagnostic(ErrorCode.ERR_NotConstantExpression, "a()").WithArguments("b").WithLocation(6, 51)
// (6,51): error CS0133: The expression being assigned to 'b' must be constant
// const Func<int> a = () => { const int b = a(); return 1; };
Diagnostic(ErrorCode.ERR_NotConstantExpression, "a()").WithArguments("b").WithLocation(6, 51)
);
}

[Fact]
public static void DoubleRecursiveConst_NotInvoked()
{
var source =
@"using System;
class C
{
public static void Main()
{
const Func<int> a = () => { const Func<int> b = a; return 1; };
}
}";
CreateCompilation(source).VerifyDiagnostics(
// (6,57): error CS0134: 'b' is of type 'Func<int>'. A const field of a reference type other than string can only be initialized with null.
// const Func<int> a = () => { const Func<int> b = a; return 1; };
Diagnostic(ErrorCode.ERR_NotNullConstRefField, "a").WithArguments("b", "System.Func<int>").WithLocation(6, 57)
);
}

Expand Down Expand Up @@ -4041,6 +4060,162 @@ public void ConstantValueFormatting()
Assert.Equal("bad", badConst.ToString(null, CultureInfo.InvariantCulture));
Assert.Equal("null", nullConst.ToString(null, CultureInfo.InvariantCulture));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_SwitchOperand()
{
var source = """
const string x = x switch { _ => "" };
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,18): error CS0110: The evaluation of the constant value for 'x' involves a circular definition
// const string x = x switch { _ => "" };
Diagnostic(ErrorCode.ERR_CircConstValue, "x").WithArguments("x").WithLocation(1, 18),
// (1,18): error CS0133: The expression being assigned to 'x' must be constant
// const string x = x switch { _ => "" };
Diagnostic(ErrorCode.ERR_NotConstantExpression, @"x switch { _ => """" }").WithArguments("x").WithLocation(1, 18));

var tree = comp.SyntaxTrees.Single();
var model = comp.GetSemanticModel(tree);
var xDeclarator = GetSyntax<VariableDeclaratorSyntax>(tree, """x = x switch { _ => "" }""");
Assert.Equal("System.String x", model.GetDeclaredSymbol(xDeclarator).ToTestDisplayString(includeNonNullable: false));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_SwitchOperand_Var()
{
var source = """
const var x = x switch { _ => "" };
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,7): error CS0822: Implicitly-typed variables cannot be constant
// const var x = x switch { _ => "" };
Diagnostic(ErrorCode.ERR_ImplicitlyTypedVariableCannotBeConst, @"var x = x switch { _ => """" }").WithLocation(1, 7),
// (1,15): error CS0841: Cannot use local variable 'x' before it is declared
// const var x = x switch { _ => "" };
Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "x").WithArguments("x").WithLocation(1, 15));

var tree = comp.SyntaxTrees.Single();
var model = comp.GetSemanticModel(tree);
var xDeclarator = GetSyntax<VariableDeclaratorSyntax>(tree, """x = x switch { _ => "" }""");
Assert.Equal("System.String x", model.GetDeclaredSymbol(xDeclarator).ToTestDisplayString(includeNonNullable: false));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_SwitchArm()
{
var source = """
const string x = 42 switch { _ => x };
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,18): error CS0133: The expression being assigned to 'x' must be constant
// const string x = 42 switch { _ => x };
Diagnostic(ErrorCode.ERR_NotConstantExpression, "42 switch { _ => x }").WithArguments("x").WithLocation(1, 18),
// (1,35): error CS0110: The evaluation of the constant value for 'x' involves a circular definition
// const string x = 42 switch { _ => x };
Diagnostic(ErrorCode.ERR_CircConstValue, "x").WithArguments("x").WithLocation(1, 35));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_Nameof()
{
var source = """
const string x = nameof(x);
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics();
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_Checked()
{
var source = """
const int x = checked(x);
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,23): error CS0110: The evaluation of the constant value for 'x' involves a circular definition
// const int x = checked(x);
Diagnostic(ErrorCode.ERR_CircConstValue, "x").WithArguments("x").WithLocation(1, 23));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_Checked_TwoConstDeclarations()
{
var source = """
const int x = checked(y);
const int y = checked(x);
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,23): error CS0841: Cannot use local variable 'y' before it is declared
// const int x = checked(y);
Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "y").WithArguments("y").WithLocation(1, 23));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_Checked_TwoConstDeclarators()
{
var source = """
const int x = y switch { _ => 42 }, y = x switch { _ => 42 };
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,15): error CS0841: Cannot use local variable 'y' before it is declared
// const int x = y switch { _ => 42 }, y = x switch { _ => 42 };
Diagnostic(ErrorCode.ERR_VariableUsedBeforeDeclaration, "y").WithArguments("y").WithLocation(1, 15),
// (1,41): error CS0133: The expression being assigned to 'y' must be constant
// const int x = y switch { _ => 42 }, y = x switch { _ => 42 };
Diagnostic(ErrorCode.ERR_NotConstantExpression, "x switch { _ => 42 }").WithArguments("y").WithLocation(1, 41));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_CollectionExpression_IEnumerable_Struct()
{
var source = """
const S x = [x];

public struct S : System.Collections.Generic.IEnumerable<S>
{
public S() => throw null;
public void Add(S s) => throw null;
System.Collections.Generic.IEnumerator<S> System.Collections.Generic.IEnumerable<S>.GetEnumerator() => throw null;
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => throw null;
}
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,7): error CS0283: The type 'S' cannot be declared const
// const S x = [x];
Diagnostic(ErrorCode.ERR_BadConstType, "S").WithArguments("S").WithLocation(1, 7));
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75353")]
public void ConstLocalCircularity_CollectionExpression_IEnumerable_Class()
{
var source = """
const S x = [x];

public class S : System.Collections.Generic.IEnumerable<S>
{
public S() => throw null;
public void Add(S s) => throw null;
System.Collections.Generic.IEnumerator<S> System.Collections.Generic.IEnumerable<S>.GetEnumerator() => throw null;
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => throw null;
}
""";
var comp = CreateCompilation(source);
comp.VerifyDiagnostics(
// (1,13): error CS0133: The expression being assigned to 'x' must be constant
// const S x = [x];
Diagnostic(ErrorCode.ERR_NotConstantExpression, "[x]").WithArguments("x").WithLocation(1, 13),
// (1,14): error CS0110: The evaluation of the constant value for 'x' involves a circular definition
// const S x = [x];
Diagnostic(ErrorCode.ERR_CircConstValue, "x").WithArguments("x").WithLocation(1, 14));
}
}

internal sealed class BoundTreeSequencer : BoundTreeWalkerWithStackGuard
Expand Down