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

Brace completion and IOperation for with expression #44712

Merged
merged 14 commits into from
Jun 6, 2020
12 changes: 12 additions & 0 deletions src/Compilers/CSharp/Portable/Operations/CSharpOperationFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ internal IOperation CreateInternal(BoundNode boundNode)
return CreateBoundDynamicIndexerAccessExpressionOperation((BoundDynamicIndexerAccess)boundNode);
case BoundKind.ObjectCreationExpression:
return CreateBoundObjectCreationExpressionOperation((BoundObjectCreationExpression)boundNode);
case BoundKind.WithExpression:
return CreateBoundWithExpressionOperation((BoundWithExpression)boundNode);
case BoundKind.DynamicObjectCreationExpression:
return CreateBoundDynamicObjectCreationExpressionOperation((BoundDynamicObjectCreationExpression)boundNode);
case BoundKind.ObjectInitializerExpression:
Expand Down Expand Up @@ -641,6 +643,16 @@ private IOperation CreateBoundObjectCreationExpressionOperation(BoundObjectCreat
return new CSharpLazyObjectCreationOperation(this, boundObjectCreationExpression, constructor.GetPublicSymbol(), _semanticModel, syntax, type, constantValue, isImplicit);
}

private IOperation CreateBoundWithExpressionOperation(BoundWithExpression boundWithExpression)
{
MethodSymbol constructor = boundWithExpression.CloneMethod;
SyntaxNode syntax = boundWithExpression.Syntax;
ITypeSymbol type = boundWithExpression.Type.GetPublicSymbol();
Optional<object> constantValue = ConvertToOptional(boundWithExpression.ConstantValue);
bool isImplicit = boundWithExpression.WasCompilerGenerated;
return new CSharpLazyWithExpressionOperation(this, boundWithExpression, constructor.GetPublicSymbol(), _semanticModel, syntax, type, constantValue, isImplicit);
}

private IDynamicObjectCreationOperation CreateBoundDynamicObjectCreationExpressionOperation(BoundDynamicObjectCreationExpression boundDynamicObjectCreationExpression)
{
ImmutableArray<string> argumentNames = boundDynamicObjectCreationExpression.ArgumentNamesOpt.NullToEmpty();
Expand Down
23 changes: 23 additions & 0 deletions src/Compilers/CSharp/Portable/Operations/CSharpOperationNodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,29 @@ protected override ImmutableArray<IArgumentOperation> CreateArguments()
}
}

internal sealed class CSharpLazyWithExpressionOperation : LazyWithExpressionOperation
{
private readonly CSharpOperationFactory _operationFactory;
private readonly BoundWithExpression _withExpression;

internal CSharpLazyWithExpressionOperation(CSharpOperationFactory operationFactory, BoundWithExpression withExpression, IMethodSymbol constructor, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue, bool isImplicit) :
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2020

Choose a reason for hiding this comment

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

constructor [](start = 142, length = 11)

Is this the clone method? The name is confusing. #Closed

base(constructor, semanticModel, syntax, type, constantValue, isImplicit)
{
_operationFactory = operationFactory;
_withExpression = withExpression;
}

protected override IObjectOrCollectionInitializerOperation CreateInitializer()
{
return (IObjectOrCollectionInitializerOperation)_operationFactory.Create(_withExpression.InitializerExpression);
}

protected override IOperation CreateValue()
{
return _operationFactory.Create(_withExpression.Receiver);
}
}

internal sealed class CSharpLazyAnonymousObjectCreationOperation : LazyAnonymousObjectCreationOperation
{
private readonly CSharpOperationFactory _operationFactory;
Expand Down
87 changes: 87 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RecordTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,6 +1119,50 @@ .maxstack 3
IL_002d: pop
IL_002e: ret
}");

var comp = verifier.Compilation;
var tree = comp.SyntaxTrees.First();
var root = tree.GetRoot();
var model = comp.GetSemanticModel(tree);

var withExpr1 = root.DescendantNodes().OfType<WithExpressionSyntax>().First();
comp.VerifyOperationTree(withExpr1, @"
IWithExpressionOperation (OperationKind.WithExpression, Type: C) (Syntax: 'c with { Y ... = W(""X"") }')
Value:
ILocalReferenceOperation: c (OperationKind.LocalReference, Type: C) (Syntax: 'c')
CloneMethod: C C.Clone()
Initializer:
IObjectOrCollectionInitializerOperation (OperationKind.ObjectOrCollectionInitializer, Type: C) (Syntax: '{ Y = W(""Y"" ... = W(""X"") }')
Initializers(2):
ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32) (Syntax: 'Y = W(""Y"")')
Left:
IPropertyReferenceOperation: System.Int32 C.Y { get; init; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'Y')
Instance Receiver:
IInstanceReferenceOperation (ReferenceKind: ImplicitReceiver) (OperationKind.InstanceReference, Type: C, IsImplicit) (Syntax: 'Y')
Right:
IInvocationOperation (System.Int32 C.W(System.String s)) (OperationKind.Invocation, Type: System.Int32) (Syntax: 'W(""Y"")')
Instance Receiver:
null
Arguments(1):
IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s) (OperationKind.Argument, Type: null) (Syntax: '""Y""')
ILiteralOperation (OperationKind.Literal, Type: System.String, Constant: ""Y"") (Syntax: '""Y""')
InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32) (Syntax: 'X = W(""X"")')
Left:
IPropertyReferenceOperation: System.Int32 C.X { get; init; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'X')
Instance Receiver:
IInstanceReferenceOperation (ReferenceKind: ImplicitReceiver) (OperationKind.InstanceReference, Type: C, IsImplicit) (Syntax: 'X')
Right:
IInvocationOperation (System.Int32 C.W(System.String s)) (OperationKind.Invocation, Type: System.Int32) (Syntax: 'W(""X"")')
Instance Receiver:
null
Arguments(1):
IArgumentOperation (ArgumentKind.Explicit, Matching Parameter: s) (OperationKind.Argument, Type: null) (Syntax: '""X""')
ILiteralOperation (OperationKind.Literal, Type: System.String, Constant: ""X"") (Syntax: '""X""')
InConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
OutConversion: CommonConversion (Exists: True, IsIdentity: True, IsNumeric: False, IsReference: False, IsUserDefined: False) (MethodSymbol: null)
");
}

[Fact]
Expand Down Expand Up @@ -1514,6 +1558,22 @@ public static void Main()
var xId = withExpr.DescendantNodes().Single(id => id.ToString() == "X");
var symbolInfo = model.GetSymbolInfo(xId);
Assert.True(x.ISymbol.Equals(symbolInfo.Symbol));

comp.VerifyOperationTree(withExpr, @"
IWithExpressionOperation (OperationKind.WithExpression, Type: C) (Syntax: 'c with { X = 2 }')
Value:
ILocalReferenceOperation: c (OperationKind.LocalReference, Type: C) (Syntax: 'c')
CloneMethod: C C.Clone()
Initializer:
IObjectOrCollectionInitializerOperation (OperationKind.ObjectOrCollectionInitializer, Type: C) (Syntax: '{ X = 2 }')
Initializers(1):
ISimpleAssignmentOperation (OperationKind.SimpleAssignment, Type: System.Int32) (Syntax: 'X = 2')
Left:
IPropertyReferenceOperation: System.Int32 C.X { get; init; } (OperationKind.PropertyReference, Type: System.Int32) (Syntax: 'X')
Instance Receiver:
IInstanceReferenceOperation (ReferenceKind: ImplicitReceiver) (OperationKind.InstanceReference, Type: C, IsImplicit) (Syntax: 'X')
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2020

Choose a reason for hiding this comment

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

IInstanceReferenceOperation (ReferenceKind: ImplicitReceiver) (OperationKind.InstanceReference, Type: C, IsImplicit) (Syntax: 'X') [](start = 18, length = 130)

Consider adjusting the comment for the node and the ReferenceKind to include information about this usage #Closed

Copy link
Member Author

@jcouv jcouv Jun 5, 2020

Choose a reason for hiding this comment

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

@AlekseyTs I didn't understand this comment. #Resolved

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't understand this comment.

IInstanceReferenceOperation and OperationKind.InstanceReference are now used in a new scenario. Consider reflecting this in the doc comment for them


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

Right:
ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2) (Syntax: '2')");
}

[Fact]
Expand Down Expand Up @@ -1555,6 +1615,33 @@ public static void Main()
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(11, 1)
);

var tree = comp.SyntaxTrees[0];
var root = tree.GetRoot();
var model = comp.GetSemanticModel(tree);

var withExpr1 = root.DescendantNodes().OfType<WithExpressionSyntax>().First();
comp.VerifyOperationTree(withExpr1, @"
IWithExpressionOperation (OperationKind.WithExpression, Type: C, IsInvalid) (Syntax: 'c with { 5 }')
Value:
ILocalReferenceOperation: c (OperationKind.LocalReference, Type: C) (Syntax: 'c')
CloneMethod: C C.Clone()
Initializer:
IObjectOrCollectionInitializerOperation (OperationKind.ObjectOrCollectionInitializer, Type: C, IsInvalid) (Syntax: '{ 5 }')
Initializers(1):
IInvalidOperation (OperationKind.Invalid, Type: System.Int32, IsInvalid, IsImplicit) (Syntax: '5')
Children(1):
ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 5, IsInvalid) (Syntax: '5')");

var withExpr2 = root.DescendantNodes().OfType<WithExpressionSyntax>().Skip(1).Single();
comp.VerifyOperationTree(withExpr2, @"
IWithExpressionOperation (OperationKind.WithExpression, Type: C, IsInvalid) (Syntax: 'c with { ')
Value:
ILocalReferenceOperation: c (OperationKind.LocalReference, Type: C) (Syntax: 'c')
CloneMethod: C C.Clone()
Initializer:
IObjectOrCollectionInitializerOperation (OperationKind.ObjectOrCollectionInitializer, Type: C, IsInvalid) (Syntax: '{ ')
Initializers(0)");
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,5 +252,7 @@ public enum OperationKind
TypePattern = 0x6f,
/// <summary>Indicates an <see cref="IRelationalPatternOperation"/>.</summary>
RelationalPattern = 0x70,
/// <summary>Indicates an <see cref="IWithExpressionOperation"/>.</summary>
WithExpression = 0x71,
}
}
95 changes: 95 additions & 0 deletions src/Compilers/Core/Portable/Generated/Operations.Generated.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2876,6 +2876,32 @@ public interface IRelationalPatternOperation : IPatternOperation
/// </summary>
IOperation Value { get; }
}
/// <summary>
/// Represents cloning of an object instance.
/// <para>
/// Current usage:
/// (1) C# with expression.
/// </para>
/// </summary>
/// <remarks>
/// This interface is reserved for implementation by its associated APIs. We reserve the right to
/// change it in the future.
/// </remarks>
public interface IWithExpressionOperation : IOperation
{
/// <summary>
/// Value to be cloned.
/// </summary>
IOperation Value { get; }
/// <summary>
/// Clone method to be invoked on the value.
/// </summary>
IMethodSymbol CloneMethod { get; }
/// <summary>
/// With collection initializer.
/// </summary>
IObjectOrCollectionInitializerOperation Initializer { get; }
}
#endregion

#region Implementations
Expand Down Expand Up @@ -8529,6 +8555,73 @@ public override IOperation Value
}
}
}
internal abstract partial class BaseWithExpressionOperation : Operation, IWithExpressionOperation
{
internal BaseWithExpressionOperation(IMethodSymbol cloneMethod, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue, bool isImplicit)
: base(OperationKind.WithExpression, semanticModel, syntax, type, constantValue, isImplicit)
{
CloneMethod = cloneMethod;
}
public abstract IOperation Value { get; }
public IMethodSymbol CloneMethod { get; }
public abstract IObjectOrCollectionInitializerOperation Initializer { get; }
public override IEnumerable<IOperation> Children
{
get
{
if (Value is object) yield return Value;
if (Initializer is object) yield return Initializer;
}
}
public override void Accept(OperationVisitor visitor) => visitor.VisitWithExpression(this);
public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> visitor, TArgument argument) => visitor.VisitWithExpression(this, argument);
}
internal sealed partial class WithExpressionOperation : BaseWithExpressionOperation, IWithExpressionOperation
{
internal WithExpressionOperation(IOperation value, IMethodSymbol cloneMethod, IObjectOrCollectionInitializerOperation initializer, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue, bool isImplicit)
: base(cloneMethod, semanticModel, syntax, type, constantValue, isImplicit)
{
Value = SetParentOperation(value, this);
Initializer = SetParentOperation(initializer, this);
}
public override IOperation Value { get; }
public override IObjectOrCollectionInitializerOperation Initializer { get; }
}
internal abstract partial class LazyWithExpressionOperation : BaseWithExpressionOperation, IWithExpressionOperation
{
private IOperation _lazyValue = s_unset;
private IObjectOrCollectionInitializerOperation _lazyInitializer = s_unsetObjectOrCollectionInitializer;
internal LazyWithExpressionOperation(IMethodSymbol cloneMethod, SemanticModel semanticModel, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue, bool isImplicit)
: base(cloneMethod, semanticModel, syntax, type, constantValue, isImplicit){ }
protected abstract IOperation CreateValue();
public override IOperation Value
{
get
{
if (_lazyValue == s_unset)
{
IOperation value = CreateValue();
SetParentOperation(value, this);
Interlocked.CompareExchange(ref _lazyValue, value, s_unset);
}
return _lazyValue;
}
}
protected abstract IObjectOrCollectionInitializerOperation CreateInitializer();
public override IObjectOrCollectionInitializerOperation Initializer
{
get
{
if (_lazyInitializer == s_unsetObjectOrCollectionInitializer)
{
IObjectOrCollectionInitializerOperation initializer = CreateInitializer();
SetParentOperation(initializer, this);
Interlocked.CompareExchange(ref _lazyInitializer, initializer, s_unsetObjectOrCollectionInitializer);
}
return _lazyInitializer;
}
}
}
#endregion
#region Visitors
public abstract partial class OperationVisitor
Expand Down Expand Up @@ -8655,6 +8748,7 @@ internal virtual void VisitNoneOperation(IOperation operation) { /* no-op */ }
public virtual void VisitBinaryPattern(IBinaryPatternOperation operation) => DefaultVisit(operation);
public virtual void VisitTypePattern(ITypePatternOperation operation) => DefaultVisit(operation);
public virtual void VisitRelationalPattern(IRelationalPatternOperation operation) => DefaultVisit(operation);
public virtual void VisitWithExpression(IWithExpressionOperation operation) => DefaultVisit(operation);
}
public abstract partial class OperationVisitor<TArgument, TResult>
{
Expand Down Expand Up @@ -8780,6 +8874,7 @@ public abstract partial class OperationVisitor<TArgument, TResult>
public virtual TResult VisitBinaryPattern(IBinaryPatternOperation operation, TArgument argument) => DefaultVisit(operation, argument);
public virtual TResult VisitTypePattern(ITypePatternOperation operation, TArgument argument) => DefaultVisit(operation, argument);
public virtual TResult VisitRelationalPattern(IRelationalPatternOperation operation, TArgument argument) => DefaultVisit(operation, argument);
public virtual TResult VisitWithExpression(IWithExpressionOperation operation, TArgument argument) => DefaultVisit(operation, argument);
}
#endregion
}
26 changes: 26 additions & 0 deletions src/Compilers/Core/Portable/Operations/OperationInterfaces.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3001,4 +3001,30 @@
</Comments>
</Property>
</Node>
<Node Name="IWithExpressionOperation" Base="IOperation" ChildrenOrder="Value,Initializer">
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 3, 2020

Choose a reason for hiding this comment

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

IWithExpressionOperation [](start = 14, length = 24)

I think we would want a special rewrite for this node in CFG, something very similar to a an object initializer operation #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 3, 2020

Choose a reason for hiding this comment

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

IWithExpressionOperation [](start = 14, length = 24)

Please run all tests with -testIOperation switch locally and confirm that none of the failures are related to the feature. #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 4, 2020

Choose a reason for hiding this comment

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

IWithExpressionOperation [](start = 14, length = 24)

I think we need to adjust OperationCloner for this node #Closed

<Comments>
<summary>
Represents cloning of an object instance.
<para>
Current usage:
(1) C# with expression.
</para>
</summary>
</Comments>
<Property Name="Value" Type="IOperation">
<Comments>
<summary>Value to be cloned.</summary>
</Comments>
</Property>
<Property Name="CloneMethod" Type="IMethodSymbol">
<Comments>
<summary>Clone method to be invoked on the value.</summary>
</Comments>
</Property>
<Property Name="Initializer" Type="IObjectOrCollectionInitializerOperation">
<Comments>
<summary>With collection initializer.</summary>
</Comments>
</Property>
</Node>
</Tree>
Loading