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

Add IOperation support for ILocalFunctionStatement #20177

Merged
merged 3 commits into from
Jun 14, 2017
Merged
Show file tree
Hide file tree
Changes from 2 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
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ private static IOperation CreateInternal(BoundNode boundNode)
return CreateBoundInterpolatedStringExpressionOperation((BoundInterpolatedString)boundNode);
case BoundKind.StringInsert:
return CreateBoundInterpolationOperation((BoundStringInsert)boundNode);
case BoundKind.LocalFunctionStatement:
return CreateBoundLocalFunctionStatementOperation((BoundLocalFunctionStatement)boundNode);
default:
var constantValue = ConvertToOptional((boundNode as BoundExpression)?.ConstantValue);
return Operation.CreateOperationNone(boundNode.HasErrors, boundNode.Syntax, constantValue, getChildren: () => GetIOperationChildren(boundNode));
Expand Down Expand Up @@ -382,6 +384,17 @@ private static ILambdaExpression CreateBoundLambdaOperation(BoundLambda boundLam
return new LazyLambdaExpression(signature, body, isInvalid, syntax, type, constantValue);
}

private static ILocalFunctionStatement CreateBoundLocalFunctionStatementOperation(BoundLocalFunctionStatement boundLocalFunctionStatement)
{
IMethodSymbol localFunctionSymbol = boundLocalFunctionStatement.Symbol;
Lazy<IBlockStatement> body = new Lazy<IBlockStatement>(() => (IBlockStatement)Create(boundLocalFunctionStatement.Body));
Copy link
Member

@333fred 333fred Jun 12, 2017

Choose a reason for hiding this comment

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

Can we assert that the body is in fact a block statement before the lazy, that way if it ever fails it'll fail in a helpful location with a better stack? #ByDesign

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we assert that the body is in fact a block statement before the lazy, that way if it ever fails it'll fail in a helpful location with a better stack?

It looks like Body is statically typed as BoundBlock.


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

bool isInvalid = boundLocalFunctionStatement.HasErrors;
SyntaxNode syntax = boundLocalFunctionStatement.Syntax;
ITypeSymbol type = null;
Optional<object> constantValue = default(Optional<object>);
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 13, 2017

Choose a reason for hiding this comment

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

Optional constantValue = default(Optional); [](start = 12, length = 59)

This and the previous statement should be pushed into the constructor, I think. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have followed the general pattern used in this file. Heejae, you might want to take care of this when we checkin the generator.


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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlekseyTs Added a note to #19720 (comment) to address this review comment.

return new LazyLocalFunctionStatement(localFunctionSymbol, body, isInvalid, syntax, type, constantValue);
}

private static IOperation CreateBoundConversionOperation(BoundConversion boundConversion)
{
ConversionKind conversionKind = GetConversionKind(boundConversion.ConversionKind);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
<Compile Include="IOperation\IOperationTests_IFieldReferenceExpression.cs" />
<Compile Include="IOperation\IOperationTests_IObjectCreationExpression.cs" />
<Compile Include="IOperation\IOperationTests_IParameterReferenceExpression.cs" />
<Compile Include="IOperation\IOperationTests_ILocalFunctionStatement.cs" />
<Compile Include="IOperation\IOperationTests_IReturnStatement.cs" />
<Compile Include="IOperation\IOperationTests_ISymbolInitializer.cs" />
<Compile Include="IOperation\IOperationTests_InvalidExpression.cs" />
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -878,19 +878,19 @@ static IEnumerable<T> MyIterator<T>(IEnumerable<T> source, Func<T, bool> predica

";
string expectedOperationTree = @"
IOperation: (OperationKind.None) (Syntax: 'IEnumerable ... }')
Children(1): IBlockStatement (2 statements) (OperationKind.BlockStatement) (Syntax: '{ ... }')
IForEachLoopStatement (Iteration variable: T element) (LoopKind.ForEach) (OperationKind.LoopStatement) (Syntax: 'foreach (va ... rn element;')
Collection: IConversionExpression (ConversionKind.Cast, Implicit) (OperationKind.ConversionExpression, Type: System.Collections.Generic.IEnumerable<T>) (Syntax: 'source')
IParameterReferenceExpression: source (OperationKind.ParameterReferenceExpression, Type: System.Collections.Generic.IEnumerable<T>) (Syntax: 'source')
Body: IIfStatement (OperationKind.IfStatement) (Syntax: 'if (predica ... rn element;')
Condition: IInvocationExpression (virtual System.Boolean System.Func<T, System.Boolean>.Invoke(T arg)) (OperationKind.InvocationExpression, Type: System.Boolean) (Syntax: 'predicate(element)')
Instance Receiver: IParameterReferenceExpression: predicate (OperationKind.ParameterReferenceExpression, Type: System.Func<T, System.Boolean>) (Syntax: 'predicate')
Arguments(1): IArgument (ArgumentKind.Explicit, Matching Parameter: arg) (OperationKind.Argument) (Syntax: 'element')
ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element')
IfTrue: IReturnStatement (OperationKind.YieldReturnStatement) (Syntax: 'yield return element;')
ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element')
IReturnStatement (OperationKind.YieldBreakStatement) (Syntax: '{ ... }')
ILocalFunctionStatement (Local Function: System.Collections.Generic.IEnumerable<T> Iterator()) (OperationKind.LocalFunctionStatement) (Syntax: 'IEnumerable ... }')
IBlockStatement (2 statements) (OperationKind.BlockStatement) (Syntax: '{ ... }')
IForEachLoopStatement (Iteration variable: T element) (LoopKind.ForEach) (OperationKind.LoopStatement) (Syntax: 'foreach (va ... rn element;')
Collection: IConversionExpression (ConversionKind.Cast, Implicit) (OperationKind.ConversionExpression, Type: System.Collections.Generic.IEnumerable<T>) (Syntax: 'source')
IParameterReferenceExpression: source (OperationKind.ParameterReferenceExpression, Type: System.Collections.Generic.IEnumerable<T>) (Syntax: 'source')
Body: IIfStatement (OperationKind.IfStatement) (Syntax: 'if (predica ... rn element;')
Condition: IInvocationExpression (virtual System.Boolean System.Func<T, System.Boolean>.Invoke(T arg)) (OperationKind.InvocationExpression, Type: System.Boolean) (Syntax: 'predicate(element)')
Instance Receiver: IParameterReferenceExpression: predicate (OperationKind.ParameterReferenceExpression, Type: System.Func<T, System.Boolean>) (Syntax: 'predicate')
Arguments(1): IArgument (ArgumentKind.Explicit, Matching Parameter: arg) (OperationKind.Argument) (Syntax: 'element')
ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element')
IfTrue: IReturnStatement (OperationKind.YieldReturnStatement) (Syntax: 'yield return element;')
ILocalReferenceExpression: element (OperationKind.LocalReferenceExpression, Type: T) (Syntax: 'element')
IReturnStatement (OperationKind.YieldBreakStatement) (Syntax: '{ ... }')
";
var expectedDiagnostics = DiagnosticDescription.None;

Expand Down
1 change: 1 addition & 0 deletions src/Compilers/Core/Portable/CodeAnalysis.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
<Compile Include="Operations\IInvocationExpression.cs" />
<Compile Include="Operations\IIsTypeExpression.cs" />
<Compile Include="Operations\ILabelStatement.cs" />
<Compile Include="Operations\ILocalFunctionStatement.cs" />
<Compile Include="Operations\ILambdaExpression.cs" />
<Compile Include="Operations\ILateBoundMemberReferenceExpression.cs" />
<Compile Include="Operations\ILiteralExpression.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4566,4 +4566,65 @@ public LazyWithStatement(Lazy<IOperation> body, Lazy<IOperation> value, bool isI
public override IOperation Value => _lazyValue.Value;
}

/// <summary>
/// Represents a local function statement.
/// </summary>
internal abstract partial class BaseLocalFunctionStatement : Operation, ILocalFunctionStatement
{
protected BaseLocalFunctionStatement(IMethodSymbol localFunctionSymbol, bool isInvalid, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue) :
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 13, 2017

Choose a reason for hiding this comment

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

ITypeSymbol type, Optional constantValue [](start = 115, length = 48)

Are these ever not null? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have used the common pattern already used in this generated file.


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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a note to #19720 (comment) to address this review comment.

base(OperationKind.LocalFunctionStatement, isInvalid, syntax, type, constantValue)
{
LocalFunctionSymbol = localFunctionSymbol;
}
/// <summary>
/// Local function symbol.
/// </summary>
public IMethodSymbol LocalFunctionSymbol { get; }
/// <summary>
/// Body of the local function.
/// </summary>
public abstract IBlockStatement Body { get; }
public override void Accept(OperationVisitor visitor)
{
visitor.VisitLocalFunctionStatement(this);
}
public override TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> visitor, TArgument argument)
{
return visitor.VisitLocalFunctionStatement(this, argument);
}
}

/// <summary>
/// Represents a local function statement.
/// </summary>
internal sealed partial class LocalFunctionStatement : BaseLocalFunctionStatement, ILocalFunctionStatement
{
public LocalFunctionStatement(IMethodSymbol localFunctionSymbol, IBlockStatement body, bool isInvalid, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue) :
base(localFunctionSymbol, isInvalid, syntax, type, constantValue)
{
Body = body;
}
/// <summary>
/// Body of the local function.
/// </summary>
public override IBlockStatement Body { get; }
}

/// <summary>
/// Represents a local function statement.
/// </summary>
internal sealed partial class LazyLocalFunctionStatement : BaseLocalFunctionStatement, ILocalFunctionStatement
{
private readonly Lazy<IBlockStatement> _lazyBody;

public LazyLocalFunctionStatement(IMethodSymbol localFunctionSymbol, Lazy<IBlockStatement> body, bool isInvalid, SyntaxNode syntax, ITypeSymbol type, Optional<object> constantValue)
: base(localFunctionSymbol, isInvalid, syntax, type, constantValue)
{
_lazyBody = body ?? throw new System.ArgumentNullException("body");
Copy link
Contributor

@AlekseyTs AlekseyTs Jun 13, 2017

Choose a reason for hiding this comment

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

"body" [](start = 71, length = 6)

Use nameof? #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 14, 2017

Choose a reason for hiding this comment

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

Use nameof?

It looks like there was no any response to this comment (#20177 (review)). #Closed

Copy link
Contributor Author

@mavasani mavasani Jun 14, 2017

Choose a reason for hiding this comment

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

I have used the existing pattern in this file. @heejaechang may want to address this in the generator. #Closed

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 14, 2017

Choose a reason for hiding this comment

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

I am confused, are you actually running a generator, or adding this code manually. If the latter, please consider addressing provided feedback. For feedback you didn't addressed because of using existing pattern, pleas open issues to follow up (applies to other similar comments). #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't address feedback as this will eventually become a generated file. I can file issues so we can address them in the generator when it is eventually checked in. Does that seem fine?

Copy link
Contributor

@AlekseyTs AlekseyTs Jun 14, 2017

Choose a reason for hiding this comment

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

I can file issues so we can address them in the generator when it is eventually checked in. Does that seem fine?

Yes, as long as we have issues tracking the debt. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AlekseyTs Added a note to #19720 (comment) to address this review comment.

}
/// <summary>
/// Body of the local function.
/// </summary>
public override IBlockStatement Body => _lazyBody.Value;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;

namespace Microsoft.CodeAnalysis.Semantics
{
/// <summary>
/// Represents a local function statement.
/// </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 ILocalFunctionStatement : IOperation
{
/// <summary>
/// Local function symbol.
/// </summary>
IMethodSymbol LocalFunctionSymbol { get; }
/// <summary>
/// Body of the local function.
/// </summary>
IBlockStatement Body { get; }
}
}

10 changes: 10 additions & 0 deletions src/Compilers/Core/Portable/Operations/OperationVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,11 @@ public virtual void VisitInvalidExpression(IInvalidExpression operation)
DefaultVisit(operation);
}

public virtual void VisitLocalFunctionStatement(ILocalFunctionStatement operation)
{
DefaultVisit(operation);
}

public virtual void VisitInterpolatedStringExpression(IInterpolatedStringExpression operation)
{
DefaultVisit(operation);
Expand Down Expand Up @@ -800,6 +805,11 @@ public virtual TResult VisitInvalidExpression(IInvalidExpression operation, TArg
return DefaultVisit(operation, argument);
}

public virtual TResult VisitLocalFunctionStatement(ILocalFunctionStatement operation, TArgument argument)
{
return DefaultVisit(operation, argument);
}

public virtual TResult VisitInterpolatedStringExpression(IInterpolatedStringExpression operation, TArgument argument)
{
return DefaultVisit(operation, argument);
Expand Down
5 changes: 5 additions & 0 deletions src/Compilers/Core/Portable/Operations/OperationWalker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,11 @@ public override void VisitLambdaExpression(ILambdaExpression operation)
Visit(operation.Body);
}

public override void VisitLocalFunctionStatement(ILocalFunctionStatement operation)
{
Visit(operation.Body);
}

public override void VisitLiteralExpression(ILiteralExpression operation)
{ }

Expand Down
6 changes: 6 additions & 0 deletions src/Compilers/Core/Portable/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,9 @@ Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression.Instance.ge
Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression.MemberName.get -> string
Microsoft.CodeAnalysis.Semantics.ILiteralExpression
Microsoft.CodeAnalysis.Semantics.ILiteralExpression.Text.get -> string
Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement
Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement.Body.get -> Microsoft.CodeAnalysis.Semantics.IBlockStatement
Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement.LocalFunctionSymbol.get -> Microsoft.CodeAnalysis.IMethodSymbol
Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression
Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression.Local.get -> Microsoft.CodeAnalysis.ILocalSymbol
Microsoft.CodeAnalysis.Semantics.ILockStatement
Expand Down Expand Up @@ -721,6 +724,7 @@ override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLabelStatement(Mi
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLambdaExpression(Microsoft.CodeAnalysis.Semantics.ILambdaExpression operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLateBoundMemberReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLiteralExpression(Microsoft.CodeAnalysis.Semantics.ILiteralExpression operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLocalFunctionStatement(Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLocalReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitLockStatement(Microsoft.CodeAnalysis.Semantics.ILockStatement operation) -> void
override Microsoft.CodeAnalysis.Semantics.OperationWalker.VisitMethodBindingExpression(Microsoft.CodeAnalysis.Semantics.IMethodBindingExpression operation) -> void
Expand Down Expand Up @@ -822,6 +826,7 @@ virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLabelStatement(Mi
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLambdaExpression(Microsoft.CodeAnalysis.Semantics.ILambdaExpression operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLateBoundMemberReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLiteralExpression(Microsoft.CodeAnalysis.Semantics.ILiteralExpression operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalFunctionStatement(Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLocalReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitLockStatement(Microsoft.CodeAnalysis.Semantics.ILockStatement operation) -> void
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor.VisitMethodBindingExpression(Microsoft.CodeAnalysis.Semantics.IMethodBindingExpression operation) -> void
Expand Down Expand Up @@ -900,6 +905,7 @@ virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.Vi
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLambdaExpression(Microsoft.CodeAnalysis.Semantics.ILambdaExpression operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLateBoundMemberReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILateBoundMemberReferenceExpression operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLiteralExpression(Microsoft.CodeAnalysis.Semantics.ILiteralExpression operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLocalFunctionStatement(Microsoft.CodeAnalysis.Semantics.ILocalFunctionStatement operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLocalReferenceExpression(Microsoft.CodeAnalysis.Semantics.ILocalReferenceExpression operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitLockStatement(Microsoft.CodeAnalysis.Semantics.ILockStatement operation, TArgument argument) -> TResult
virtual Microsoft.CodeAnalysis.Semantics.OperationVisitor<TArgument, TResult>.VisitMethodBindingExpression(Microsoft.CodeAnalysis.Semantics.IMethodBindingExpression operation, TArgument argument) -> TResult
Expand Down
11 changes: 11 additions & 0 deletions src/Test/Utilities/Portable/Compilation/OperationTreeVerifier.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,17 @@ public override void VisitIfStatement(IIfStatement operation)
Visit(operation.IfFalseStatement, "IfFalse");
}

public override void VisitLocalFunctionStatement(ILocalFunctionStatement operation)
{
LogString(nameof(ILocalFunctionStatement));

LogSymbol(operation.LocalFunctionSymbol, header: " (Local Function");
LogString(")");
LogCommonPropertiesAndNewLine(operation);

Visit(operation.Body);
}

private void LogCaseClauseCommon(ICaseClause operation)
{
var kindStr = $"{nameof(CaseKind)}.{operation.CaseKind}";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,13 @@ public override void VisitLambdaExpression(ILambdaExpression operation)
base.VisitLambdaExpression(operation);
}

public override void VisitLocalFunctionStatement(ILocalFunctionStatement operation)
{
var localFunction = operation.LocalFunctionSymbol;

base.VisitLocalFunctionStatement(operation);
}

public override void VisitLiteralExpression(ILiteralExpression operation)
{
var text = operation.Text;
Expand Down