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.

Sign up for GitHub

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

Jump to bottom

Make local function parameters bind lazily #16736

Closed
wants to merge 2 commits into from
Closed

Make local function parameters bind lazily #16736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dfc6aba
Make local function parameters lazier
agocke Jan 16, 2017
580f286
Respond to PR comments
agocke Jan 25, 2017
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
1 change: 0 additions & 1 deletion src/Compilers/CSharp/Portable/CSharpCodeAnalysis.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,7 +657,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
13 changes: 11 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/Source/LocalFunctionSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ public LocalFunctionSymbol(
/// </summary>
internal Binder ScopeBinder { get; }

/// <summary>
/// Binder used to bind local function parameters.
/// </summary>
internal Binder ParameterBinder => _binder;

internal void GetDeclarationDiagnostics(DiagnosticBag addTo)
{
// Force attribute binding for diagnostics
Expand All @@ -119,6 +124,11 @@ internal void GetDeclarationDiagnostics(DiagnosticBag addTo)
// force lazy init
ComputeParameters();

foreach (var param in _lazyParametersAndDiagnostics.Parameters)
{
((SourceParameterSymbol)param).GetDeclarationDiagnostics(addTo);
}

foreach (var p in _syntax.ParameterList.Parameters)
{
if (p.IsArgList)
Expand Down Expand Up @@ -184,8 +194,7 @@ private void ComputeParameters()
_syntax.ParameterList,
allowRefOrOut: true,
arglistToken: out arglistToken,
diagnostics: diagnostics,
beStrict: true);
diagnostics: diagnostics);

var isVararg = (arglistToken.Kind() == SyntaxKind.ArgListKeyword);
if (IsAsync && diagnostics.IsEmptyWithoutResolution)
Expand Down
6 changes: 2 additions & 4 deletions src/Compilers/CSharp/Portable/Symbols/Source/ParameterHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ public static ImmutableArray<ParameterSymbol> MakeParameters(
BaseParameterListSyntax syntax,
bool allowRefOrOut,
out SyntaxToken arglistToken,
DiagnosticBag diagnostics,
bool beStrict)
DiagnosticBag diagnostics)
{
arglistToken = default(SyntaxToken);

Expand Down Expand Up @@ -78,8 +77,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
48 changes: 31 additions & 17 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceComplexParameterSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ private enum ParameterSyntaxKind : byte

private CustomAttributesBag<CSharpAttributeData> _lazyCustomAttributesBag;
private ThreeState _lazyHasOptionalAttribute;
protected ConstantValue _lazyDefaultSyntaxValue;
private ConstantValue _lazyDefaultSyntaxValue;

private Binder LocalFunctionParameterBinder
=> (ContainingSymbol as LocalFunctionSymbol)?.ParameterBinder;

internal SourceComplexParameterSymbol(
Symbol owner,
Expand Down Expand Up @@ -69,11 +72,6 @@ internal SourceComplexParameterSymbol(
_lazyDefaultSyntaxValue = defaultSyntaxValue;
}

protected virtual Binder ParameterBinder
{
get { return null; }
}

internal override SyntaxReference SyntaxReference
{
get
Expand Down Expand Up @@ -112,7 +110,7 @@ internal override ConstantValue ExplicitDefaultConstantValue
// Dev11 emits the first parameter as option with default value and the second as regular parameter.
// The syntactic default value is suppressed since additional synthesized parameters are added at the end of the signature.

return DefaultSyntaxValue ?? DefaultValueFromAttributes;
return GetDefaultSyntaxValue() ?? DefaultValueFromAttributes;
}
}

Expand Down Expand Up @@ -194,23 +192,28 @@ internal override bool IsCallerMemberName
}
}

private ConstantValue DefaultSyntaxValue
private ConstantValue GetDefaultSyntaxValue(DiagnosticBag diagnosticsOpt = null)
Copy link
Contributor

@AlekseyTs AlekseyTs Jan 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private ConstantValue GetDefaultSyntaxValue(DiagnosticBag diagnosticsOpt = null) [](start = 8, length = 80)

Please open an issue to get rid of this pattern post RTM #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think this is encompassed by #16652?

That would remove the flakiness with adding to declaration diagnostics.

Copy link
Contributor

@AlekseyTs AlekseyTs Jan 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat, I think we should explicitly track API changes what we would like to revert. We can keep the list in that issue. #Closed

{
get
{
if (_lazyDefaultSyntaxValue == ConstantValue.Unset)
if (_lazyDefaultSyntaxValue == ConstantValue.Unset)
{
var diagnostics = DiagnosticBag.GetInstance();
if (Interlocked.CompareExchange(
ref _lazyDefaultSyntaxValue,
MakeDefaultExpression(diagnostics, LocalFunctionParameterBinder),
ConstantValue.Unset)
== ConstantValue.Unset)
{
var diagnostics = DiagnosticBag.GetInstance();
if (Interlocked.CompareExchange(ref _lazyDefaultSyntaxValue, MakeDefaultExpression(diagnostics, ParameterBinder), ConstantValue.Unset) == ConstantValue.Unset)
if (diagnosticsOpt == null)
{
AddDeclarationDiagnostics(diagnostics);
}

diagnostics.Free();
}
diagnosticsOpt?.AddRange(diagnostics);

return _lazyDefaultSyntaxValue;
diagnostics.Free();
}

return _lazyDefaultSyntaxValue;
}

// If binder is null, then get it from the compilation. Otherwise use the provided binder.
Expand Down Expand Up @@ -453,7 +456,7 @@ internal sealed override CustomAttributesBag<CSharpAttributeData> GetAttributesB
else
{
var attributeSyntax = this.GetAttributeDeclarations();
bagCreatedOnThisThread = LoadAndValidateAttributes(attributeSyntax, ref _lazyCustomAttributesBag, addToDiagnostics: diagnosticsOpt, binderOpt: ParameterBinder);
bagCreatedOnThisThread = LoadAndValidateAttributes(attributeSyntax, ref _lazyCustomAttributesBag, addToDiagnostics: diagnosticsOpt, binderOpt: LocalFunctionParameterBinder);
}

if (bagCreatedOnThisThread)
Expand Down Expand Up @@ -1054,6 +1057,17 @@ public override ImmutableArray<CustomModifier> RefCustomModifiers
}
}

/// <summary>
/// Force diagnostics to be realized and added to the bag
/// </summary>
internal override void GetDeclarationDiagnostics(DiagnosticBag diagnostics)
{
base.GetDeclarationDiagnostics(diagnostics);

// Force default value
GetDefaultSyntaxValue(diagnostics);
}

internal override void ForceComplete(SourceLocation locationOpt, CancellationToken cancellationToken)
{
base.ForceComplete(locationOpt, cancellationToken);
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ protected override void MethodChecks(DiagnosticBag diagnostics)
var bodyBinder = binderFactory.GetBinder(parameterList, syntax, this).WithContainingMemberOrLambda(this);

SyntaxToken arglistToken;
_lazyParameters = ParameterHelpers.MakeParameters(bodyBinder, this, parameterList, true, out arglistToken, diagnostics, false);
_lazyParameters = ParameterHelpers.MakeParameters(
bodyBinder, this, parameterList, true, out arglistToken, diagnostics);
_lazyIsVararg = (arglistToken.Kind() == SyntaxKind.ArgListKeyword);
_lazyReturnType = bodyBinder.GetSpecialType(SpecialType.System_Void, diagnostics, syntax);

Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceDelegateMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ internal InvokeMethod(
this.refKind = refKind;

SyntaxToken arglistToken;
var parameters = ParameterHelpers.MakeParameters(binder, this, syntax.ParameterList, true, out arglistToken, diagnostics, false);
var parameters = ParameterHelpers.MakeParameters(
binder, this, syntax.ParameterList, true, out arglistToken, diagnostics);
if (arglistToken.Kind() == SyntaxKind.ArgListKeyword)
{
// This is a parse-time error in the native compiler; it is a semantic analysis error in Roslyn.
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ private void MethodChecks(MethodDeclarationSyntax syntax, Binder withTypeParamsB
// instance). Constraints are checked in AfterAddingTypeMembersChecks.
var signatureBinder = withTypeParamsBinder.WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.SuppressConstraintChecks, this);

_lazyParameters = ParameterHelpers.MakeParameters(signatureBinder, this, syntax.ParameterList, true, out arglistToken, diagnostics, false);
_lazyParameters = ParameterHelpers.MakeParameters(
signatureBinder, this, syntax.ParameterList, true, out arglistToken, diagnostics);
_lazyIsVararg = (arglistToken.Kind() == SyntaxKind.ArgListKeyword);
RefKind refKind;
var returnTypeSyntax = syntax.ReturnType.SkipRef(out refKind);
Expand Down
26 changes: 7 additions & 19 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceParameterSymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,7 @@ public static SourceParameterSymbol Create(
int ordinal,
bool isParams,
bool isExtensionMethodThis,
DiagnosticBag diagnostics,
bool beStrict)
DiagnosticBag diagnostics)
{
var name = identifier.ValueText;
var locations = ImmutableArray.Create<Location>(new SourceLocation(identifier));
Expand All @@ -60,23 +59,6 @@ public static SourceParameterSymbol Create(
return new SourceSimpleParameterSymbol(owner, parameterType, ordinal, refKind, name, locations);
}

if (beStrict)
{
return new SourceStrictComplexParameterSymbol(
diagnostics,
context,
owner,
ordinal,
parameterType,
refKind,
name,
locations,
syntax.GetReference(),
ConstantValue.Unset,
isParams,
isExtensionMethodThis);
}

return new SourceComplexParameterSymbol(
owner,
ordinal,
Expand Down Expand Up @@ -160,6 +142,12 @@ internal override void ForceComplete(SourceLocation locationOpt, CancellationTok
state.DefaultForceComplete(this);
}

internal virtual void GetDeclarationDiagnostics(DiagnosticBag diagnosticsOpt = null)
{
// Force attributes
GetAttributesBag(diagnosticsOpt);
}

/// <summary>
/// True if the parameter is marked by <see cref="System.Runtime.InteropServices.OptionalAttribute"/>.
/// </summary>
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/CSharp/Portable/Symbols/Source/SourcePropertySymbol.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,8 @@ private static ImmutableArray<ParameterSymbol> MakeParameters(Binder binder, Sou
}

SyntaxToken arglistToken;
var parameters = ParameterHelpers.MakeParameters(binder, owner, parameterSyntaxOpt, false, out arglistToken, diagnostics, false);
var parameters = ParameterHelpers.MakeParameters(
binder, owner, parameterSyntaxOpt, false, out arglistToken, diagnostics);

if (arglistToken.Kind() != SyntaxKind.None)
{
Expand Down
44 changes: 0 additions & 44 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceStrictComplexParameterSymbol.cs
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions src/Compilers/CSharp/Portable/Symbols/Source/SourceUserDefinedOperatorSymbolBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,8 +130,7 @@ protected override void MethodChecks(DiagnosticBag diagnostics)
ParameterListSyntax,
true,
out arglistToken,
diagnostics,
false);
diagnostics);

if (arglistToken.Kind() == SyntaxKind.ArgListKeyword)
{
Expand Down
21 changes: 21 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,27 @@ public static IMethodSymbol FindLocalFunction(this CommonTestBase.CompilationVer
[CompilerTrait(CompilerFeature.LocalFunctions)]
public class CodeGenLocalFunctionTests : CSharpTestBase
{
[Fact]
public void NameofRecursiveDefaultParameter()
{
var comp = CreateCompilationWithMscorlib(@"
using System;
class C
{
public static void Main()
{
void Local(string s = nameof(Local))
{
Console.WriteLine(s);
}
Local();
}
}", options: TestOptions.ReleaseExe);
comp.VerifyDiagnostics();
comp.DeclarationDiagnostics.Verify();
CompileAndVerify(comp, expectedOutput: "Local");
}

[Fact]
[WorkItem(16399, "https://github.com/dotnet/roslyn/issues/16399")]
public void RecursiveGenericLocalFunctionIterator()
Expand Down
Loading