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

Put record parameters in scope withing instance initializers #44906

Merged
merged 5 commits into from
Jun 6, 2020
Merged

Put record parameters in scope withing instance initializers #44906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
544916f
Put record parameters in scope withing instance initializers
AlekseyTs Jun 5, 2020
edab8c7
Merge remote-tracking branch 'dotnet/features/records' into Records_04
AlekseyTs Jun 6, 2020
f96f815
Follow-up on merge
AlekseyTs Jun 6, 2020
1cc4a0a
Share closure across initializers and the base call.
AlekseyTs Jun 6, 2020
2b0ca70
Merge remote-tracking branch 'dotnet/features/records' into Records_04
AlekseyTs Jun 6, 2020
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
19 changes: 16 additions & 3 deletions src/Compilers/CSharp/Portable/Binder/Binder_Initializers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,15 +106,13 @@ internal static void BindRegularCSharpFieldInitializers(
}

Binder parentBinder = binderFactory.GetBinder(initializerNode);
Debug.Assert((parentBinder.ContainingMemberOrLambda is TypeSymbol containing && TypeSymbol.Equals(containing, fieldSymbol.ContainingType, TypeCompareKind.ConsiderEverything2)) || //should be the binder for the type
fieldSymbol.ContainingType.IsImplicitClass); //however, we also allow fields in namespaces to help support script scenarios

if (firstDebugImports == null)
{
firstDebugImports = parentBinder.ImportChain;
}

parentBinder = new LocalScopeBinder(parentBinder).WithAdditionalFlagsAndContainingMemberOrLambda(BinderFlags.FieldInitializer, fieldSymbol);
parentBinder = parentBinder.GetFieldInitializerBinder(fieldSymbol);

BoundFieldEqualsValue boundInitializer = BindFieldInitializer(parentBinder, fieldSymbol, initializerNode, diagnostics);
boundInitializers.Add(boundInitializer);
Expand Down Expand Up @@ -145,6 +143,21 @@ internal static void BindRegularCSharpFieldInitializers(
}
}

internal Binder GetFieldInitializerBinder(FieldSymbol fieldSymbol, bool suppressBinderFlagsFieldInitializer = false)
{
Debug.Assert((ContainingMemberOrLambda is TypeSymbol containing && TypeSymbol.Equals(containing, fieldSymbol.ContainingType, TypeCompareKind.ConsiderEverything2)) || //should be the binder for the type
fieldSymbol.ContainingType.IsImplicitClass); //however, we also allow fields in namespaces to help support script scenarios

Binder binder = this;

if (!fieldSymbol.IsStatic && fieldSymbol.ContainingType.GetMembersUnordered().OfType<SynthesizedRecordConstructor>().SingleOrDefault() is SynthesizedRecordConstructor recordCtor)
{
binder = new InMethodBinder(recordCtor, binder);
}

return new LocalScopeBinder(binder).WithAdditionalFlagsAndContainingMemberOrLambda(suppressBinderFlagsFieldInitializer ? BinderFlags.None : BinderFlags.FieldInitializer, fieldSymbol);
}

/// <summary>
/// In script C#, some field initializers are assignments to fields and others are global
/// statements. There are no restrictions on accessing instance members.
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Binder/ExpressionVariableFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public override void VisitConstructorDeclaration(ConstructorDeclarationSyntax no

public override void VisitRecordDeclaration(RecordDeclarationSyntax node)
{
Debug.Assert(((RecordDeclarationSyntax)node).ParameterList is object);
Debug.Assert(node.ParameterList is object);

if (node.BaseWithArguments is SimpleBaseTypeSyntax baseWithArguments)
{
Expand Down
9 changes: 1 addition & 8 deletions src/Compilers/CSharp/Portable/Compilation/SyntaxTreeSemanticModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1261,15 +1261,8 @@ private FieldSymbol GetDeclaredFieldSymbol(VariableDeclaratorSyntax variableDecl

private Binder GetFieldOrPropertyInitializerBinder(FieldSymbol symbol, Binder outer, EqualsValueClauseSyntax initializer)
{
BinderFlags flags = BinderFlags.None;

// NOTE: checking for a containing script class is sufficient, but the regular C# test is quick and easy.
if (this.IsRegularCSharp || !symbol.ContainingType.IsScriptClass)
{
flags |= BinderFlags.FieldInitializer;
}

outer = new LocalScopeBinder(outer).WithAdditionalFlagsAndContainingMemberOrLambda(flags, symbol);
outer = outer.GetFieldInitializerBinder(symbol, suppressBinderFlagsFieldInitializer: !this.IsRegularCSharp && symbol.ContainingType.IsScriptClass);

if (initializer != null)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ private void CompileMethod(
analyzedInitializers = InitializerRewriter.RewriteConstructor(processedInitializers.BoundInitializers, methodSymbol);
processedInitializers.HasErrors = processedInitializers.HasErrors || analyzedInitializers.HasAnyErrors;

if (body != null && ((methodSymbol.ContainingType.IsStructType() && !methodSymbol.IsImplicitConstructor) || _emitTestCoverageData))
if (body != null && ((methodSymbol.ContainingType.IsStructType() && !methodSymbol.IsImplicitConstructor) || methodSymbol is SynthesizedRecordConstructor || _emitTestCoverageData))
{
if (_emitTestCoverageData && methodSymbol.IsImplicitConstructor)
{
Expand Down
110 changes: 110 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LookupPositionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2043,6 +2043,116 @@ interface C : Base(X)
TestLookupNames(text, expectedNames);
}

[Fact]
public void RecordInitializers_01()
{
var text = @"
record C(int X)
`{
int Z `= X + 1`;
`}
";
var expectedNames = MakeExpectedSymbols(
Add( // Global
"System",
"Microsoft",
"C"),
Add( // Members
"System.Boolean C.Equals(C? )",
"System.Boolean C.Equals(System.Object? )",
"System.Boolean System.Object.Equals(System.Object obj)",
"System.Boolean System.Object.Equals(System.Object objA, System.Object objB)",
"System.Boolean System.Object.ReferenceEquals(System.Object objA, System.Object objB)",
"System.Int32 C.GetHashCode()",
"System.Int32 C.X { get; init; }",
"System.Int32 C.Z",
"System.Int32 System.Object.GetHashCode()",
"System.Object System.Object.MemberwiseClone()",
"System.String System.Object.ToString()",
"System.Type C.EqualityContract { get; }",
"System.Type System.Object.GetType()",
"void System.Object.Finalize()"),
Combine(
Remove("System.Int32 C.X { get; init; }"),
Add("System.Int32 X")
),
Combine(s_pop, s_pop),
s_pop
);

TestLookupNames(text, expectedNames);
}

[Fact]
public void RecordInitializers_02()
{
var text = @"
record C(int X)
`{
static int Z = X + 1;
`}
";
var expectedNames = MakeExpectedSymbols(
Add( // Global
"System",
"Microsoft",
"C"),
Add( // Members
"System.Boolean C.Equals(C? )",
"System.Boolean C.Equals(System.Object? )",
"System.Boolean System.Object.Equals(System.Object obj)",
"System.Boolean System.Object.Equals(System.Object objA, System.Object objB)",
"System.Boolean System.Object.ReferenceEquals(System.Object objA, System.Object objB)",
"System.Int32 C.GetHashCode()",
"System.Int32 C.X { get; init; }",
"System.Int32 C.Z",
"System.Int32 System.Object.GetHashCode()",
"System.Object System.Object.MemberwiseClone()",
"System.String System.Object.ToString()",
"System.Type C.EqualityContract { get; }",
"System.Type System.Object.GetType()",
"void System.Object.Finalize()"),
s_pop
);

TestLookupNames(text, expectedNames);
}

[Fact]
public void RecordInitializers_03()
{
var text = @"
record C(int X)
`{
const int Z = X + 1;
`}
";
var expectedNames = MakeExpectedSymbols(
Add( // Global
"System",
"Microsoft",
"C"),
Add( // Members
"System.Boolean C.Equals(C? )",
"System.Boolean C.Equals(System.Object? )",
"System.Boolean System.Object.Equals(System.Object obj)",
"System.Boolean System.Object.Equals(System.Object objA, System.Object objB)",
"System.Boolean System.Object.ReferenceEquals(System.Object objA, System.Object objB)",
"System.Int32 C.GetHashCode()",
"System.Int32 C.X { get; init; }",
"System.Int32 C.Z",
"System.Int32 System.Object.GetHashCode()",
"System.Object System.Object.MemberwiseClone()",
"System.String System.Object.ToString()",
"System.Type C.EqualityContract { get; }",
"System.Type System.Object.GetType()",
"void System.Object.Finalize()"),
s_pop
);

TestLookupNames(text, expectedNames);
}
Copy link
Member

@cston cston Jun 6, 2020 •
edited by AlekseyTs
Loading

Choose a reason for hiding this comment

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

Consider testing a const field as well. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Consider testing a const field as well.

Const fields are static fields, so I am pretty confident this test is fairly equivalent to verify scoping rules. I will add a test like that for completeness sake in a separate PR

In reply to: 436281327 [](ancestors = 436281327)


/// <summary>
/// Given a program, calls LookupNames at each character position and verifies the results.
///
Expand Down
Loading