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

Implement binding and lowering for the with expression #43249

Merged
merged 8 commits into from
May 1, 2020
Merged
Show file tree
Hide file tree
Changes from 6 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
3 changes: 3 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -684,6 +684,9 @@ private BoundExpression BindExpressionInternal(ExpressionSyntax node, Diagnostic
case SyntaxKind.SuppressNullableWarningExpression:
return BindSuppressNullableWarningExpression((PostfixUnaryExpressionSyntax)node, diagnostics);

case SyntaxKind.WithExpression:
return BindWithExpression((WithExpressionSyntax)node, diagnostics);

default:
// NOTE: We could probably throw an exception here, but it's conceivable
// that a non-parser syntax tree could reach this point with an unexpected
Expand Down
175 changes: 175 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable enable

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.CSharp
{
/// <summary>
/// This portion of the binder converts a <see cref="WithExpressionSyntax"/> into a <see cref="BoundExpression"/>.
/// </summary>
internal partial class Binder
{
private BoundExpression BindWithExpression(WithExpressionSyntax syntax, DiagnosticBag diagnostics)
{
// PROTOTYPE: this entire method is likely to change
var receiver = BindRValueWithoutTargetType(syntax.Receiver, diagnostics);
var receiverType = receiver.Type;

var lookupResult = LookupResult.GetInstance();
HashSet<DiagnosticInfo>? useSiteDiagnostics = null;
bool hasErrors = false;

if (receiverType is null || receiverType.IsVoidType())
{
diagnostics.Add(ErrorCode.ERR_InvalidWithReceiverType, syntax.Receiver.Location);
receiverType = CreateErrorType();
}

MethodSymbol? cloneMethod = null;
if (!receiverType.IsErrorType())
{
// PROTOTYPE: The receiver type must have a instance method called 'Clone' with no parameters
LookupMembersInType(
lookupResult,
receiverType,
"Clone",
agocke marked this conversation as resolved.
Show resolved Hide resolved
arity: 0,
ConsList<TypeSymbol>.Empty,
LookupOptions.MustBeInstance | LookupOptions.MustBeInvocableIfMember,
this,
diagnose: false,
ref useSiteDiagnostics);

if (lookupResult.IsMultiViable)
{
foreach (var symbol in lookupResult.Symbols)
{
if (symbol is MethodSymbol { ParameterCount: 0 } m)
{
cloneMethod = m;
break;
Copy link
Member

@gafter gafter Apr 29, 2020

Choose a reason for hiding this comment

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

How does this handle multiple results? That can happen for example when Clone is overridden or hides another. This technique does not correspond to how invocations are handled, as there is no contract for the order of symbols in a lookup result. #Resolved

}
}
}

lookupResult.Clear();
// PROTOTYPE: discarding use-site diagnostics
useSiteDiagnostics = null;

if (cloneMethod is null)
{
hasErrors = true;
diagnostics.Add(ErrorCode.ERR_NoSingleCloneMethod, syntax.Receiver.Location, receiverType);
}
else
{
// Check return type
if (!receiverType.IsEqualToOrDerivedFrom(
cloneMethod.ReturnType,
TypeCompareKind.ConsiderEverything,
ref useSiteDiagnostics))
{
hasErrors = true;
diagnostics.Add(
ErrorCode.ERR_ContainingTypeMustDeriveFromWithReturnType,
syntax.Receiver.Location,
receiverType,
cloneMethod.ReturnType);
}

// PROTOTYPE: discarding use-site diagnostics
useSiteDiagnostics = null;
}
}

var cloneReturnType = cloneMethod?.ReturnType;

var args = ArrayBuilder<(Symbol?, BoundExpression)>.GetInstance();
// Bind with expression arguments
foreach (var initializer in syntax.Initializers)
{
var propName = initializer.NameEquals?.Name.Identifier.Text;
Symbol? member = null;
if (!(propName is null) && !(cloneReturnType is null))
{
var location = initializer.NameEquals!.Name.Location;
this.LookupMembersInType(
lookupResult,
cloneReturnType,
propName,
arity: 0,
basesBeingResolved: null,
options: LookupOptions.Default,
originalBinder: this,
diagnose: false,
useSiteDiagnostics: ref useSiteDiagnostics);
if (lookupResult.IsSingleViable &&
Copy link
Member

@gafter gafter Apr 29, 2020

Choose a reason for hiding this comment

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

IsSingleViable [](start = 37, length = 14)

How does this handle properties that are overridden or hidden (more than one symbol in the lookup result)? This does not correspond to how expressions are bound. #Closed

lookupResult.SingleSymbolOrDefault is var sym)
{
switch (sym.Kind)
{
case SymbolKind.Property:
member = sym;
// PROTOTYPE: this should check for init-only, but that isn't a separate feature yet
// It also will not work in metadata.
if (!(sym is SynthesizedRecordPropertySymbol))
333fred marked this conversation as resolved.
Show resolved Hide resolved
agocke marked this conversation as resolved.
Show resolved Hide resolved
{
goto default;
333fred marked this conversation as resolved.
Show resolved Hide resolved
}
break;

default:
hasErrors = true;
diagnostics.Add(
ErrorCode.ERR_WithMemberIsNotRecordProperty,
location);
break;
}
}

if (!hasErrors && member is null)
{
hasErrors = true;
Error(
diagnostics,
ErrorCode.ERR_NoSuchMemberOrExtension,
location,
cloneReturnType,
propName);
}
}

var expr = BindValue(initializer.Expression, diagnostics, BindValueKind.RValue);
if (!(member is null))
{
expr = GenerateConversionForAssignment(
member.GetTypeOrReturnType().Type,
expr,
diagnostics);
}
lookupResult.Clear();
args.Add((member, expr));
}

lookupResult.Free();

return new BoundWithExpression(
syntax,
receiver,
cloneMethod,
args.ToImmutableAndFree(),
cloneReturnType ?? receiverType,
hasErrors: hasErrors);
}
}
}
11 changes: 11 additions & 0 deletions src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2052,4 +2052,15 @@
<Field Name="Type" Type="TypeSymbol?" Override="true"/> <!-- We use null Type for placeholders representing out vars -->
<Field Name="NullableAnnotation" Type="NullableAnnotation"/>
</Node>

<Node Name="BoundWithExpression" Base="BoundExpression">
<!-- Non-null type is required for this node kind -->
<Field Name="Type" Type="TypeSymbol" Override="true" Null="disallow"/>
<Field Name="Receiver" Type="BoundExpression" />
<!-- CloneMethod may be null in error scenarios-->
<Field Name="CloneMethod" Type="MethodSymbol?" />
<!-- Members and expressions passed as arguments to the With expression. -->
<Field Name="Arguments" Type="ImmutableArray&lt;(Symbol? Member, BoundExpression Expression)&gt;" />
</Node>

</Tree>
28 changes: 20 additions & 8 deletions src/Compilers/CSharp/Portable/CSharpResources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -6046,6 +6046,18 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_ExpressionTreeContainsRangeExpression" xml:space="preserve">
<value>An expression tree may not contain a range ('..') expression.</value>
</data>
<data name="WRN_GeneratorFailedDuringGeneration" xml:space="preserve">
<value>Generator '{0}' failed to generate source. It will not contribute to the output and compilation errors may occur as a result.</value>
</data>
<data name="WRN_GeneratorFailedDuringInitialization" xml:space="preserve">
<value>Generator '{0}' failed to initialize. It will not contribute to the output and compilation errors may occur as a result.</value>
</data>
<data name="WRN_GeneratorFailedDuringGeneration_Title" xml:space="preserve">
<value>Generator failed to generate source.</value>
</data>
<data name="WRN_GeneratorFailedDuringInitialization_Title" xml:space="preserve">
<value>Generator failed to initialize.</value>
</data>
<data name="IDS_FeatureRecords" xml:space="preserve">
<value>records</value>
</data>
Expand All @@ -6055,16 +6067,16 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
<data name="ERR_DuplicateRecordConstructor" xml:space="preserve">
<value>There cannot be a primary constructor and a member constructor with the same parameter types.</value>
</data>
<data name="WRN_GeneratorFailedDuringGeneration" xml:space="preserve">
<value>Generator '{0}' failed to generate source. It will not contribute to the output and compilation errors may occur as a result.</value>
<data name="ERR_InvalidWithReceiverType" xml:space="preserve">
<value>The receiver of a `with` expression must have a non-void type.</value>
</data>
<data name="WRN_GeneratorFailedDuringInitialization" xml:space="preserve">
<value>Generator '{0}' failed to initialize. It will not contribute to the output and compilation errors may occur as a result.</value>
<data name="ERR_NoSingleCloneMethod" xml:space="preserve">
<value>The receiver type '{0}' does not have an accessible parameterless instance method named "Clone".</value>
</data>
<data name="WRN_GeneratorFailedDuringGeneration_Title" xml:space="preserve">
<value>Generator failed to generate source.</value>
<data name="ERR_ContainingTypeMustDeriveFromWithReturnType" xml:space="preserve">
<value>The type of the 'with' expression receiver, '{0}', does not derive from the return type of the 'Clone' method, '{1}'.</value>
</data>
<data name="WRN_GeneratorFailedDuringInitialization_Title" xml:space="preserve">
<value>Generator failed to initialize.</value>
<data name="ERR_WithMemberIsNotRecordProperty" xml:space="preserve">
<value>All arguments to a `with` expression must be compiler-generated record properties.</value>
</data>
</root>
4 changes: 4 additions & 0 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1775,6 +1775,10 @@ internal enum ErrorCode

ERR_BadRecordDeclaration = 8800,
ERR_DuplicateRecordConstructor = 8801,
ERR_InvalidWithReceiverType = 8802,
ERR_NoSingleCloneMethod = 8803,
ERR_ContainingTypeMustDeriveFromWithReturnType = 8804,
ERR_WithMemberIsNotRecordProperty = 8808,

// Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2024,6 +2024,12 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node)
return null;
}

public override BoundNode VisitWithExpression(BoundWithExpression node)
{
// PROTOTYPE: This is wrong
return null;
}

public override BoundNode VisitArrayAccess(BoundArrayAccess node)
{
VisitRvalue(node.Expression);
Expand Down
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2222,6 +2222,15 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node)
return base.VisitWhileStatement(node);
}

public override BoundNode VisitWithExpression(BoundWithExpression expr)
{
// PROTOTYPE: This is wrong
SetResultType(expr,
expr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState()
?? TypeWithState.Create(expr.Type, NullableFlowState.NotNull));
return null;
}

public override BoundNode VisitForStatement(BoundForStatement node)
{
DeclareLocals(node.OuterLocals);
Expand Down
Loading