Skip to content

Commit

Permalink
Make local function default parameter value binding lazy
Browse files Browse the repository at this point in the history 
Current strict binding can cause circularity problems when local
functions are referenced. This change causes local functions to use lazy
default parameter binding, similar to methods, and then forces their
construction when diagnostics are requested for the local function.

This also requires a mechanism for recording declaration diagnostics
outside of adding to the compilation's DeclarationDiagnostics. A new
type, DeclarationDiagnosticStore is introduced as an abstraction to
store declaration diagnostics on either the compilation or in a local
DiagnosticBag, depending on the needs of the symbol storing diagnostics.

Fixes dotnet#16451
  • Loading branch information
@agocke
agocke committed Mar 6, 2017
1 parent c1fc264 commit 3da2068
Show file tree
Hide file tree
Showing 18 changed files with 239 additions and 275 deletions.
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,6 @@
<Compile Include="Symbols\Source\SourcePropertyAccessorSymbol.cs" />
<Compile Include="Symbols\Source\SourcePropertySymbol.cs" />
<Compile Include="Symbols\Source\SourceSimpleParameterSymbol.cs" />
<Compile Include="Symbols\Source\SourceStrictComplexParameterSymbol.cs" />
<Compile Include="Symbols\Source\SourceTypeParameterSymbol.cs" />
<Compile Include="Symbols\Source\SourceUserDefinedConversionSymbol.cs" />
<Compile Include="Symbols\Source\SourceUserDefinedOperatorSymbol.cs" />
Expand Down
177 changes: 76 additions & 101 deletions src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,55 +13,24 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
internal sealed class LocalFunctionSymbol : MethodSymbol
{
private sealed class ParametersAndDiagnostics
{
internal readonly ImmutableArray<ParameterSymbol> Parameters;
internal readonly bool IsVararg;
internal readonly ImmutableArray<Diagnostic> Diagnostics;

internal ParametersAndDiagnostics(ImmutableArray<ParameterSymbol> parameters, bool isVararg, ImmutableArray<Diagnostic> diagnostics)
{
Parameters = parameters;
IsVararg = isVararg;
Diagnostics = diagnostics;
}
}

private sealed class TypeParameterConstraintsAndDiagnostics
{
internal readonly ImmutableArray<TypeParameterConstraintClause> ConstraintClauses;
internal readonly ImmutableArray<Diagnostic> Diagnostics;

internal TypeParameterConstraintsAndDiagnostics(ImmutableArray<TypeParameterConstraintClause> constraintClauses, ImmutableArray<Diagnostic> diagnostics)
{
ConstraintClauses = constraintClauses;
Diagnostics = diagnostics;
}
}

private sealed class ReturnTypeAndDiagnostics
{
internal readonly TypeSymbol ReturnType;
internal readonly ImmutableArray<Diagnostic> Diagnostics;

internal ReturnTypeAndDiagnostics(TypeSymbol returnType, ImmutableArray<Diagnostic> diagnostics)
{
ReturnType = returnType;
Diagnostics = diagnostics;
}
}

private readonly Binder _binder;
private readonly LocalFunctionStatementSyntax _syntax;
private readonly Symbol _containingSymbol;
private readonly DeclarationModifiers _declarationModifiers;
private readonly ImmutableArray<LocalFunctionTypeParameterSymbol> _typeParameters;
private readonly RefKind _refKind;
private ParametersAndDiagnostics _lazyParametersAndDiagnostics;
private TypeParameterConstraintsAndDiagnostics _lazyTypeParameterConstraintsAndDiagnostics;
private ReturnTypeAndDiagnostics _lazyReturnTypeAndDiagnostics;

private ImmutableArray<ParameterSymbol> _lazyParameters;
private bool _lazyIsVarArg;
private ImmutableArray<TypeParameterConstraintClause> _lazyTypeParameterConstraints;
private TypeSymbol _lazyReturnType;
private TypeSymbol _iteratorElementType;
private ImmutableArray<Diagnostic> _diagnostics;

// Lock for initializing lazy fields and registering their diagnostics
// Acquire this lock when initializing lazy objects to guarantee their declaration
// diagnostics get added to the store exactly once
private readonly DiagnosticBag _declarationDiagnostics;

public LocalFunctionSymbol(
Binder binder,
Expand All @@ -76,31 +45,31 @@ public LocalFunctionSymbol(
DeclarationModifiers.Static |
syntax.Modifiers.ToDeclarationModifiers();

var diagnostics = DiagnosticBag.GetInstance();

ScopeBinder = binder;

binder = binder.WithUnsafeRegionIfNecessary(syntax.Modifiers);

_declarationDiagnostics = new DiagnosticBag();

if (_syntax.TypeParameterList != null)
{
binder = new WithMethodTypeParametersBinder(this, binder);
_typeParameters = MakeTypeParameters(diagnostics);
_typeParameters = MakeTypeParameters(_declarationDiagnostics);
}
else
{
_typeParameters = ImmutableArray<LocalFunctionTypeParameterSymbol>.Empty;
ReportErrorIfHasConstraints(_syntax.ConstraintClauses, diagnostics);
ReportErrorIfHasConstraints(_syntax.ConstraintClauses, _declarationDiagnostics);
}

if (IsExtensionMethod)
{
diagnostics.Add(ErrorCode.ERR_BadExtensionAgg, Locations[0]);
_declarationDiagnostics.Add(ErrorCode.ERR_BadExtensionAgg, Locations[0]);
}

_binder = binder;
_refKind = (syntax.ReturnType.Kind() == SyntaxKind.RefType) ? RefKind.Ref : RefKind.None;
_diagnostics = diagnostics.ToReadOnlyAndFree();

}

/// <summary>
Expand All @@ -109,49 +78,39 @@ public LocalFunctionSymbol(
/// </summary>
internal Binder ScopeBinder { get; }

public Binder ParameterBinder => _binder;

internal void GetDeclarationDiagnostics(DiagnosticBag addTo)
{
// Force attribute binding for diagnostics
// Force complete type parameters
foreach (var typeParam in _typeParameters)
{
typeParam.GetAttributesBag(addTo);
typeParam.ForceComplete(null, default(CancellationToken));
}

// force lazy init
ComputeParameters();

foreach (var p in _syntax.ParameterList.Parameters)
foreach (var p in _lazyParameters)
{
if (p.IsArgList)
{
addTo.Add(ErrorCode.ERR_IllegalVarArgs, p.Location);
}
// Force complete parameters to retrieve all diagnostics
p.ForceComplete(null, default(CancellationToken));
}

ComputeReturnType();

var diags = ImmutableInterlocked.InterlockedExchange(ref _diagnostics, default(ImmutableArray<Diagnostic>));
if (!diags.IsDefault)
{
addTo.AddRange(diags);
addTo.AddRange(_lazyParametersAndDiagnostics.Diagnostics);
// Note _lazyParametersAndDiagnostics and _lazyReturnTypeAndDiagnostics
// are computed always, but _lazyTypeParameterConstraintsAndDiagnostics
// is only computed if there are constraints.
if (_lazyTypeParameterConstraintsAndDiagnostics != null)
{
addTo.AddRange(_lazyTypeParameterConstraintsAndDiagnostics.Diagnostics);
}
addTo.AddRange(_lazyReturnTypeAndDiagnostics.Diagnostics);
}
addTo.AddRange(_declarationDiagnostics);
}

internal override void AddDeclarationDiagnostics(DiagnosticBag diagnostics)
=> _declarationDiagnostics.AddRange(diagnostics);

public override bool IsVararg
{
get
{
ComputeParameters();
return _lazyParametersAndDiagnostics.IsVararg;
return _lazyIsVarArg;
}
}

Expand All @@ -160,49 +119,51 @@ public override ImmutableArray<ParameterSymbol> Parameters
get
{
ComputeParameters();
return _lazyParametersAndDiagnostics.Parameters;
return _lazyParameters;
}
}

private void ComputeParameters()
{
if (_lazyParametersAndDiagnostics != null)
if (_lazyParameters != null)
{
return;
}

var diagnostics = DiagnosticBag.GetInstance();
SyntaxToken arglistToken;

foreach (var param in _syntax.ParameterList.Parameters)
{
ReportAnyAttributes(diagnostics, param.AttributeLists);
}
var diagnostics = DiagnosticBag.GetInstance();

var parameters = ParameterHelpers.MakeParameters(
_binder,
this,
_syntax.ParameterList,
arglistToken: out arglistToken,
diagnostics: diagnostics,
allowRefOrOut: true,
allowThis: true,
beStrict: true);
diagnostics: diagnostics);

var isVararg = arglistToken.Kind() == SyntaxKind.ArgListKeyword;
if (isVararg)
{
diagnostics.Add(ErrorCode.ERR_IllegalVarArgs, arglistToken.GetLocation());
}

var isVararg = (arglistToken.Kind() == SyntaxKind.ArgListKeyword);
if (IsAsync && diagnostics.IsEmptyWithoutResolution)
if (IsAsync)
{
SourceMemberMethodSymbol.ReportAsyncParameterErrors(parameters, diagnostics, this.Locations[0]);
}
var value = new ParametersAndDiagnostics(parameters, isVararg, diagnostics.ToReadOnlyAndFree());
Interlocked.CompareExchange(ref _lazyParametersAndDiagnostics, value, null);
}

private static void ReportAnyAttributes(DiagnosticBag diagnostics, SyntaxList<AttributeListSyntax> attributes)
{
foreach (var attrList in attributes)
lock (_declarationDiagnostics)
{
diagnostics.Add(ErrorCode.ERR_AttributesInLocalFuncDecl, attrList.Location);
if (_lazyParameters != null)
{
diagnostics.Free();
return;
}

_declarationDiagnostics.AddRangeAndFree(diagnostics);
_lazyIsVarArg = isVararg;
_lazyParameters = parameters;
}
}

Expand All @@ -211,7 +172,7 @@ public override TypeSymbol ReturnType
get
{
ComputeReturnType();
return _lazyReturnTypeAndDiagnostics.ReturnType;
return _lazyReturnType;
}
}

Expand All @@ -225,7 +186,7 @@ internal override RefKind RefKind

internal void ComputeReturnType()
{
if (_lazyReturnTypeAndDiagnostics != null)
if (_lazyReturnType != null)
{
return;
}
Expand All @@ -243,13 +204,21 @@ internal void ComputeReturnType()
diagnostics.Add(ErrorCode.ERR_BadAsyncReturn, this.Locations[0]);
}

if (refKind != RefKind.None && returnType.SpecialType == SpecialType.System_Void)
Debug.Assert(refKind == RefKind.None
|| returnType.SpecialType != SpecialType.System_Void
|| returnTypeSyntax.HasErrors);

lock (_declarationDiagnostics)
{
Debug.Assert(returnTypeSyntax.HasErrors);
}
if (_lazyReturnType != null)
{
diagnostics.Free();
return;
}

var value = new ReturnTypeAndDiagnostics(returnType, diagnostics.ToReadOnlyAndFree());
Interlocked.CompareExchange(ref _lazyReturnTypeAndDiagnostics, value, null);
_declarationDiagnostics.AddRangeAndFree(diagnostics);
_lazyReturnType = returnType;
}
}

public override bool ReturnsVoid => ReturnType?.SpecialType == SpecialType.System_Void;
Expand Down Expand Up @@ -387,8 +356,6 @@ private ImmutableArray<LocalFunctionTypeParameterSymbol> MakeTypeParameters(Diag
var location = identifier.GetLocation();
var name = identifier.ValueText;

ReportAnyAttributes(diagnostics, parameter.AttributeLists);

foreach (var @param in result)
{
if (name == @param.Name)
Expand Down Expand Up @@ -432,15 +399,23 @@ internal ImmutableArray<TypeSymbol> GetTypeParameterConstraintTypes(int ordinal)

private TypeParameterConstraintClause GetTypeParameterConstraintClause(int ordinal)
{
if (_lazyTypeParameterConstraintsAndDiagnostics == null)
if (_lazyTypeParameterConstraints == null)
{
var diagnostics = DiagnosticBag.GetInstance();
var constraints = MakeTypeParameterConstraints(diagnostics);
var value = new TypeParameterConstraintsAndDiagnostics(constraints, diagnostics.ToReadOnlyAndFree());
Interlocked.CompareExchange(ref _lazyTypeParameterConstraintsAndDiagnostics, value, null);

lock (_declarationDiagnostics)
{
if (_lazyTypeParameterConstraints == null)
{
_declarationDiagnostics.AddRange(diagnostics);
_lazyTypeParameterConstraints = constraints;
}
}
diagnostics.Free();
}

var clauses = _lazyTypeParameterConstraintsAndDiagnostics.ConstraintClauses;
var clauses = _lazyTypeParameterConstraints;
return (clauses.Length > 0) ? clauses[ordinal] : null;
}

Expand Down
17 changes: 13 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@ public static ImmutableArray<ParameterSymbol> MakeParameters(
out SyntaxToken arglistToken,
DiagnosticBag diagnostics,
bool allowRefOrOut,
bool allowThis,
bool beStrict)
bool allowThis)
{
arglistToken = default(SyntaxToken);

Expand Down Expand Up @@ -85,8 +84,7 @@ public static ImmutableArray<ParameterSymbol> MakeParameters(
parameterIndex,
(paramsKeyword.Kind() != SyntaxKind.None),
parameterIndex == 0 && thisKeyword.Kind() != SyntaxKind.None,
diagnostics,
beStrict);
diagnostics);

ReportParameterErrors(owner, parameterSyntax, parameter, firstDefault, diagnostics);

Expand Down Expand Up @@ -259,6 +257,17 @@ private static void ReportParameterErrors(
}
}

public static void ReportAttributesDisallowed(this LocalFunctionSymbol localFunc, SyntaxList<AttributeListSyntax> attributes)
{
var diagnostics = DiagnosticBag.GetInstance();
foreach (var attrList in attributes)
{
diagnostics.Add(ErrorCode.ERR_AttributesInLocalFuncDecl, attrList.Location);
}
localFunc.AddDeclarationDiagnostics(diagnostics);
diagnostics.Free();
}

internal static bool ReportDefaultParameterErrors(
Binder binder,
Symbol owner,
Expand Down
Loading

0 comments on commit 3da2068

Please sign in to comment.