Ensure that we get a well formed child IOperation tree even for bound… #18976

mavasani · 2017-04-25T12:14:29Z

… nodes for whom IOperation support is not yet implemented

Customer scenario

Due to large number of unimplemented IOperation APIs for new language and existing features, all the IOperation analyzers (such as FXCop analyzers) do not receive any operation callbacks for code containing these features (tuples, string interpolation, query expressions, object and collection initializers, etc.). This causes lot of false positives in the analyzer diagnostics as it doesn't see parameter or field references within such code. Additionally, the analyzers do not report true issues in expressions within this language features. This is one of the biggest blocker towards dogfooding the FXCop analyzers.

Bugs this fixes:

Fixes #8884

Workarounds, if any

Disable the rule or suppress all the false positives. All our three repos consuming the FXCop analyzers (Roslyn, Project system and Roslyn-analyzers) were forced to disable these rules due to extremely large number of false positives.

Risk

Low. Proposed fix is to introduce a temporary internal interface IOperationWithChildren for bound nodes for which we haven't designed the IOperation APIs or they have missing pieces. This will allow the OperationWalker to walk the descendants of such nodes and make analyzer callbacks and drastically reduce the false positives. This change doesn't modify the publically available IOperation tree or the API, so we should be able to remove it once we have fully designed and implemented IOperation (tracked by #19060)

Performance impact

Low. We will now walk additional IOperation nodes and make callbacks, but this is a functional requirement.

Is this a regression from a previous update?

No.

Root cause analysis:

IOperation based analyzers are still being developed and tested.

How was the bug found?

Lots of customer reports about false positives.

… nodes for whom IOperation support is not yet implemented This ensures that the analyzer driver/operation walker is able to find all the descendant IOperation nodes within these not yet implemented features. We should remove the IOperationWithChildren interface once we have designed all the IOperation APIs. TODO: Implement the VB part. Fixes dotnet#8884

mavasani · 2017-04-25T12:14:51Z

Tagging @dotnet/analyzer-ioperation for review.

mavasani · 2017-04-26T22:29:52Z

Added the VB support and tests.

Tagging @AlekseyTs @cston as well.

AlekseyTs · 2017-04-27T18:14:56Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

+            var collectionInitializerExpresion = objectOrCollectionInitializer as BoundCollectionInitializerExpression;
+            if (collectionInitializerExpresion != null)
+            {
+                return collectionInitializerExpresion.Initializers;


return collectionInitializerExpresion.Initializers; [](start = 16, length = 51)

Is this new code path covered by tests? #Closed

I have added a skipped test for this. It still doesn't work as expected due to the fact that IObjectCreationExpression.Initializers is still returning ImmutableArray of ISymbolInitializers.

I am not comfortable with changes that affect behavior of existing APIs and are not covered by active tests. I think we should either revert changes to IObjectCreationExpression.MemberInitializers implementation, or cover the changes in tests. #Closed

Makes sense, let me revert the changes to IObjectCreationExpression.MemberInitializers implementation.

AlekseyTs · 2017-04-27T18:24:03Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1001,6 +1016,8 @@ internal sealed partial class BoundDeconstructionAssignmentOperator : BoundExpre
        // TODO: implement IOperation for pattern-matching constructs (https://github.com/dotnet/roslyn/issues/8699)
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Left, this.Right);


this.Right [](start = 111, length = 10)

We might not want to expose the right node as is. For example, I believe it can contain special conversions that probably shouldn't be exposed to the outside world in that form. I think we might want to expose only the nodes that aren't special for deconstruction. CC @jcouv #Closed

Note that the IOperationWithChildren API is just internal, which only the OperationWalker understands so we can walk down it's children when computing all operations to analyze in a method block. Public clients will still not be able to access any children nodes, we don't need to worry about exposing unwanted nodes here.

AlekseyTs · 2017-04-27T18:25:28Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1413,6 +1420,8 @@ internal partial class BoundDynamicIndexerAccess
    {
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => this.Arguments.Add(this.ReceiverOpt).As<IOperation>();


this.ReceiverOpt [](start = 85, length = 16)

Receiver should probably be the first. We also probably don't want to add nulls to the list. #Closed

Given that the API is only internal, and the OperationWalker handles null child nodes inside Children, I felt it is better to add all nodes and the client handle null cases.

I felt it is better to add all nodes and the client handle null cases.

Then we should clearly document this on the API in the BoundNode and in the interface. #Closed

AlekseyTs · 2017-04-27T18:31:27Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1488,6 +1503,8 @@ internal partial class BoundRangeVariable
    {
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Value);


protected override ImmutableArray Children => ImmutableArray.Create(this.Value); [](start = 8, length = 104)

This doesn't feel right. It looks like the value doesn't correspond to anything explicitly present in source. #Closed

Okay, I will revert this.

AlekseyTs · 2017-04-27T19:17:18Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1533,6 +1550,8 @@ internal partial class BoundQueryClause
    {
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Value);


protected override ImmutableArray Children => ImmutableArray.Create(this.Value); [](start = 8, length = 104)

I am not sure if we want to expose the entire value here. I think it will include calls to the query operators, but at the design meeting I didn't get an impression that we would want to expose them right now. #Closed

How do we ensure that we make callbacks for any operation nodes for user code within query? I think the fact that we are returning compiler generated nodes is irrelevant to this approach or the IOperationWithChildren API.

AlekseyTs · 2017-04-27T19:19:27Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

+
+    internal partial class BoundDeclarationPattern
+    {
+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.VariableAccess, this.DeclaredType);


protected override ImmutableArray Children => ImmutableArray.Create(this.VariableAccess, this.DeclaredType); [](start = 7, length = 133)

Should be an empty set, I think. #Closed

Okay, I can revert

AlekseyTs · 2017-04-27T19:32:44Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1818,6 +1873,8 @@ internal partial class BoundDynamicInvocation
    {
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => this.Arguments.As<IOperation>().Add(this.Expression);


this.Arguments.As().Add(this.Expression); [](start = 66, length = 53)

Expression should probably be first. #Closed

Order doesn't really matter as we are not exposing this publically.

AlekseyTs · 2017-04-27T19:35:16Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

@@ -1833,6 +1890,8 @@ internal partial class BoundArrayLength
    {
        protected override OperationKind ExpressionKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Expression);


protected override ImmutableArray Children => ImmutableArray.Create(this.Expression); [](start = 8, length = 109)

I think we can skip this node, it is created only at lowering. #Closed

AlekseyTs · 2017-04-27T20:27:43Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

@@ -767,6 +795,8 @@ internal partial class BoundConditionalGoto
    {
        protected override OperationKind StatementKind => OperationKind.None;

+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Condition);


protected override ImmutableArray Children => ImmutableArray.Create(this.Condition); [](start = 8, length = 108)

This is a lower level node. #Closed

AlekseyTs · 2017-04-27T20:42:53Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

+                {
+                    builder.Add(section);
+                }
+                builder.Add(this.DefaultLabel);


builder.Add(this.DefaultLabel); [](start = 16, length = 31)

It feels like one of the sections refers to this node and this member is just for compiler's convenience. #Closed

Okay, in that case I can remove this statement.

Should be verified with a test. #Closed

AlekseyTs · 2017-04-27T20:46:17Z

src/Compilers/CSharp/Portable/Lowering/AsyncRewriter/AwaitExpressionSpiller.cs

@@ -58,6 +58,8 @@ public bool HasLocals

            protected override OperationKind ExpressionKind => OperationKind.None;

+            protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Value);


protected override ImmutableArray Children => ImmutableArray.Create(this.Value); [](start = 12, length = 104)

This is a lower node, I think we shouldn't touch it. #Closed

AlekseyTs · 2017-04-27T20:55:16Z

src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs

+  Children(2): IOperation:  (OperationKind.None) (Syntax: 'var (x, y)')
+      Children(2): ILocalReferenceExpression: x (OperationKind.LocalReferenceExpression, Type: System.Int32) (Syntax: 'x')
+        ILocalReferenceExpression: y (OperationKind.LocalReferenceExpression, Type: System.Int32) (Syntax: 'y')
+    IConversionExpression (ConversionKind.Invalid, Implicit) (OperationKind.ConversionExpression, Type: (System.Int32, System.Int32)) (Syntax: 'point')


IConversionExpression (ConversionKind.Invalid, Implicit) (OperationKind.ConversionExpression, Type: (System.Int32, System.Int32)) (Syntax: 'point') [](start = 4, length = 147)

This looks like a conversion that we might not want to expose. #Closed

AlekseyTs · 2017-04-27T20:56:52Z

src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs

+        }
+
+        [Fact, WorkItem(8884, "https://github.com/dotnet/roslyn/issues/8884")]
+        public void ParameterReference_PatternSwitchStatement()


ParameterReference_PatternSwitchStatement [](start = 20, length = 41)

It looks like we are missing a test with default label. #Closed

AlekseyTs · 2017-04-27T23:34:36Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

BoundArrayLength is lower node, there is no need to modify it. #Closed

AlekseyTs · 2017-04-27T23:37:25Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

It looks like BoundReferenceAssignment is a lower node, we don't need to modify it. #Closed

AlekseyTs · 2017-04-27T23:49:55Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)
+            Get
+                Return ImmutableArray.Create(Of IOperation)(Me.OriginalArgument, Me.InConversion, Me.InPlaceholder, Me.OutConversion, Me.OutPlaceholder)


Me.InConversion, Me.InPlaceholder, Me.OutConversion, Me.OutPlaceholder [](start = 81, length = 70)

I think we shouldn't expose these nodes for now. Placeholders are going to be duplicated in conversions. #Closed

OriginalArgument is fine.

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

AlekseyTs · 2017-04-27T23:52:00Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

BoundLateBoundArgumentSupportingAssignmentWithCapture looks like a lower node, no need to modify it. #Closed

AlekseyTs · 2017-04-27T23:59:48Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)
+            Get
+                Return ImmutableArray.Create(Of IOperation)(Me.CapturedGroupOpt, Me.GroupPlaceholderOpt, Me.UnderlyingExpression)


Me.GroupPlaceholderOpt [](start = 81, length = 22)

I think GroupPlaceholderOpt should be excluded from the list, it simply provides identity. #Closed

AlekseyTs · 2017-04-28T00:05:35Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

        Protected Overrides Function ExpressionKind() As OperationKind
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

BoundComplexConditionalAccessReceiver looks like a lower node, there is no need to modify it. #Closed

AlekseyTs · 2017-04-28T17:13:22Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

+        /// </summary>
+        protected virtual ImmutableArray<IOperation> Children => ImmutableArray<IOperation>.Empty;
+
+        public virtual void Accept(OperationVisitor visitor)


public virtual void Accept(OperationVisitor visitor) [](start = 8, length = 52)

Should this be abstract instead? #Closed

Never mind, looks fine.

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

AlekseyTs · 2017-04-28T17:13:41Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

+            visitor.VisitNoneOperation(this);
+        }
+
+        public virtual TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> visitor, TArgument argument)


public virtual TResult Accept<TArgument, TResult>(OperationVisitor<TArgument, TResult> visitor, TArgument argument) [](start = 8, length = 115)

Should this be abstract instead? #Closed

Never mind, looks fine.

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

AlekseyTs · 2017-04-28T17:24:43Z

src/Compilers/VisualBasic/Portable/BoundTree/Statement.vb

@@ -1714,6 +1794,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

BoundConditionalGoto is a lower level node, no need to modify it. #Closed

AlekseyTs · 2017-04-28T17:26:53Z

src/Compilers/VisualBasic/Portable/BoundTree/Statement.vb

@@ -1806,6 +1910,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)


Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation) [](start = 8, length = 79)

BoundUnstructuredExceptionOnErrorSwitch is a lower level node, no need to modify it. #Closed

AlekseyTs · 2017-05-01T23:48:29Z

Done with review pass. #Closed

mavasani · 2017-05-03T16:27:49Z

@AlekseyTs The PR is ready for another review. Thanks!

AlekseyTs · 2017-05-03T17:55:51Z

src/Compilers/CSharp/Portable/BoundTree/Expression.cs

+
+    internal partial class BoundDeclarationPattern
+    {
+        protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.VariableAccess);


protected override ImmutableArray Children => ImmutableArray.Create(this.VariableAccess); [](start = 8, length = 113)

I do not understand why is this changed back? #Closed

Aleksey, this was breaking a test where we do not get an ILocalReferenceExpression for the variable access.

https://github.com/dotnet/roslyn/pull/18976/files#diff-779f42c6a7094b52a59f494a27a6ec7aR568

I guess, it is fine to expose that node until we figure out how declaration pattern is supposed to be represented. #Closed

AlekseyTs · 2017-05-03T17:57:38Z

    protected override ImmutableArray<IOperation> Children => this.Arguments.As<IOperation>();

Not sure why this code is removed. #Closed

Refers to: src/Compilers/CSharp/Portable/BoundTree/Expression.cs:1587 in 5f6e036. [](commit_id = 5f6e036, deletion_comment = True)

mavasani · 2017-05-03T18:01:46Z

Not sure why this code is removed.

I deleted it accidentally in one of the previous commits, and I have added it back in the latest commit.

AlekseyTs · 2017-05-03T18:07:22Z

I deleted it accidentally in one of the previous commits, and I have added it back in the latest commit.

It looks like it wasn't added back, at least I don't see it. #Closed

AlekseyTs · 2017-05-03T18:25:10Z

src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_IParameterReferenceExpression.cs

+                references: new[] { MscorlibRef, SystemRef, compilation0.EmitToImageReference(embedInteropTypes: true) });
+
+            string expectedOperationTree = @"
+IOperation:  (OperationKind.None) (Syntax: 'new I(x)')


It feels like we should have a node for x. #Closed

Yes, but the generated bound node doesn't seem to have it. I will file an issue.

Filed #19219

AlekseyTs · 2017-05-03T19:27:12Z

src/Compilers/VisualBasic/Portable/BoundTree/Expression.vb

@@ -2375,6 +2426,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
            Return OperationKind.None
        End Function

+        Protected Overrides ReadOnly Property Children As ImmutableArray(Of IOperation)
+            Get
+                Return ImmutableArray.Create(Of IOperation)(Me.OriginalArgument)


Return ImmutableArray.Create(Of IOperation)(Me.OriginalArgument) [](start = 16, length = 64)

Is this code path covered by tests? #Closed

AlekseyTs · 2017-05-03T19:28:17Z

Done with review pass. #Closed

… as it is never exposed in the operation tree

mavasani · 2017-05-03T21:05:09Z

Also tagging @cston for review.

AlekseyTs

LGTM

mavasani · 2017-05-03T22:55:32Z

@ManishJayaswal @Pilchie for ask mode approval.

cston · 2017-05-04T16:58:36Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

@@ -9,23 +9,52 @@

 namespace Microsoft.CodeAnalysis.CSharp
 {
-    internal partial class BoundStatement : IOperation
+    internal partial class BoundNode : IOperation, IOperationWithChildren


Is there an advantage to having a separate interface for IOperationWithChildren? Aren't callers forced to check for an implementation of IOperationWithChildren, then check .Children.IsEmpty or perhaps even .IsDefault?

@cston This is supposed to be a temporary internal interface that will exist until will we finish IOperation API design and implementation. Only the OperationWalker is aware about this interface to enable the analyzer driver to walk the descendants under unimplemented bound nodes. Having a special interface allows us to then just delete the interface and verify/fix up bound nodes still relying on it.

cston · 2017-05-04T17:11:06Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

+        {
+            get
+            {
+                var builder = ImmutableArray.CreateBuilder<IOperation>(this.SwitchSections.Length + 2);


this.SwitchSections.Length + 1?

cston · 2017-05-04T17:12:38Z

src/Compilers/CSharp/Portable/BoundTree/Statement.cs

+
+    partial class BoundPatternSwitchSection
+    {
+        protected override ImmutableArray<IOperation> Children => this.SwitchLabels.As<IOperation>().AddRange(this.Statements).ToImmutableArray();


AddRange() returns an ImmutableArray<T>. No need for .ToImmutableArray().

cston · 2017-05-04T17:24:41Z

LGTM

mavasani · 2017-05-04T18:33:01Z

FYI @heejaechang - once this PR is merged, it will cause merge conflicts in the IOperation feature branch due to your OperationFactory work. We can sit together and resolve these conflicts.

mavasani · 2017-05-04T18:45:13Z

@dotnet-bot retest windows_debug_unit32_prtest please

heejaechang · 2017-05-04T18:45:49Z

@mavasani sure.

mavasani · 2017-05-04T19:41:20Z

@dotnet-bot retest windows_debug_unit32_prtest please

mavasani · 2017-05-04T19:42:49Z

@dotnet-bot retest windows_release_vs-integration_prtest please

dnfclas added the cla-already-signed label Apr 25, 2017

VB side changes and unit tests for IOperationWithChildren

6ecf157

mavasani added Area-Analyzers Area-Compilers labels Apr 26, 2017

AlekseyTs reviewed Apr 27, 2017

View reviewed changes

AlekseyTs reviewed Apr 28, 2017

View reviewed changes

mavasani added 2 commits May 2, 2017 16:30

Add more unit tests

ed668bc

Fix a unit test failure

1152ab7

AlekseyTs reviewed May 3, 2017

View reviewed changes

mavasani mentioned this pull request May 3, 2017

BoundNoPiaObjectCreationExpression should expose arguments as child bound nodes #19219

Closed

Add arglist test and revert changes to BoundByRefArgumentWithCopyBack…

47b42a3

… as it is never exposed in the operation tree

AlekseyTs approved these changes May 3, 2017

View reviewed changes

ManishJayaswal added the Approved to merge label May 3, 2017

cston reviewed May 4, 2017

View reviewed changes

Address PR feedback from Chuck

084cc62

mavasani merged commit 22a3dd6 into dotnet:master May 4, 2017

mavasani deleted the IOperationWithChildren branch May 4, 2017 20:20

mavasani mentioned this pull request May 5, 2017

IParameterReferenceExpression operation not generated for parameter access in a certain cases #8884

Closed

		@@ -58,6 +58,8 @@ public bool HasLocals

		protected override OperationKind ExpressionKind => OperationKind.None;

		protected override ImmutableArray<IOperation> Children => ImmutableArray.Create<IOperation>(this.Value);

Ensure that we get a well formed child IOperation tree even for bound… #18976

Ensure that we get a well formed child IOperation tree even for bound… #18976

Conversation

mavasani commented Apr 25, 2017 • edited Loading

mavasani commented Apr 25, 2017

mavasani commented Apr 26, 2017

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 28, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented May 1, 2017 • edited Loading

mavasani commented May 3, 2017

AlekseyTs May 3, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 3, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented May 3, 2017 • edited Loading

mavasani commented May 3, 2017

mavasani commented Apr 25, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 27, 2017 •

edited

Loading

AlekseyTs Apr 28, 2017 •

edited

Loading

AlekseyTs Apr 28, 2017 •

edited

Loading

AlekseyTs Apr 28, 2017 •

edited

Loading

AlekseyTs Apr 28, 2017 •

edited

Loading

AlekseyTs Apr 28, 2017 •

edited

Loading

AlekseyTs commented May 1, 2017 •

edited

Loading

AlekseyTs May 3, 2017 •

edited

Loading

AlekseyTs May 3, 2017 •

edited

Loading

AlekseyTs commented May 3, 2017 •

edited

Loading

AlekseyTs commented May 3, 2017 •

edited

Loading

AlekseyTs May 3, 2017 •

edited

Loading

AlekseyTs May 3, 2017 •

edited

Loading

AlekseyTs commented May 3, 2017 •

edited

Loading