Skip to content

Commit

Permalink
Fix circularity issue with binding const local (#75371)
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv authored Oct 11, 2024
1 parent d5a5b21 commit d1afb2f
Show file tree
Hide file tree
Showing 4 changed files with 226 additions and 13 deletions.
13 changes: 12 additions & 1 deletion src/Compilers/CSharp/Portable/Binder/LocalBinderFactory.cs
View file
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
View file
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;
}
}

internal void SetLocalSymbol(LocalSymbol local)
{
Interlocked.CompareExchange(ref _localSymbol, local, null);
}
}
}
22 changes: 20 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceLocalSymbol.cs
View file
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
View file
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

0 comments on commit d1afb2f

Please sign in to comment.