Don't require ValueTuple type in deconstruction if return value is not used #20295

jcouv · 2017-06-17T03:23:36Z

Deconstructions without tuples in the right-hand-side, such as (x, y) = e;, shouldn't require the ValueTuple types, because the only tuple involved is the return value.

This change has two parts: (1) binding, and (2) lowering.
When binding a deconstruction, we can ignore the errors that come with the return type, when the return value is not used. We know the return value is not used in an expression-statement, and the initializer and incrementor parts of a for-statement.
When lowering the deconstruction we ignore the return value. It does not get lowered itself and gets replaced by a BoundVoid for emit stage.

This maintains the behavior of the semantic model: the type of the whole deconstruction expression is still a tuple type. But its underlying type may be an error type if the ValueTuple types were missing.

Fixes #18629

jcouv · 2017-06-17T22:09:32Z

@cston I think the solution you suggested mostly works, with some additional tweaks in lowering. But there is one issue left with EE.

In CompilationContext.BindExpression we (1) bind the expression as part of an expression-statement, and then (2) bind the resulting bound value into a BoundReturnStatement.

The problem is that step (1) will consider that the return value isn't used, but when the bound node from step (2) reaches the emit stage, the emitter sees that the value is used.

I've tried a simple modification of step (1) to wrap the expression in a return-statement. But that won't work for expressions like (var x, var y) = e which are only valid in a statement.

Update: From discussion with Chuck, I will try modifying CompilationContext.BindExpression to handle deconstructions directly (recognize the case, call BindDeconstruction, passing a flag).

Related tests:

ExpressionEvaluator.UnitTests.DeclarationTests.DeconstructionDeclarationWithDiscard
ExpressionEvaluator.UnitTests.DeclarationTests.DeconstructionDeclaration #Resolved

jcouv · 2017-06-20T19:20:01Z

@cston @dotnet/roslyn-compiler for review. Thanks
This change removes the requirement for ValueTuple type when using a deconstruction only using the Deconstruct method, when we can tell that the return value won't be used (expression-statement, foreach, for-incrementor). #Resolved

cston · 2017-06-20T21:29:31Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

+            return new BoundDeconstructionAssignmentOperator(node, lhsTuple, boundConversion, resultIsUsed, returnType);
+        }
+
+        private bool IsDeconstructionResultUsed(ExpressionSyntax left)


static #Resolved

cston · 2017-06-20T21:31:25Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

+        private bool IsDeconstructionResultUsed(ExpressionSyntax left)
+        {
+            var parent = left.Parent;
+            if (parent is null || left.Parent.Kind() == SyntaxKind.ForEachVariableStatement)


|| parent.Kind() == ... #Resolved

cston · 2017-06-20T22:00:46Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

@@ -1095,6 +1090,119 @@ public void Deconstruct(out int a, out int b)
        }

        [Fact]
+        public void ValueTupleNotRequiredIfReturnIsUsed()


Should this be ...IfReturnIsNotUsed? #Resolved

cston · 2017-06-20T22:07:26Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

@@ -116,9 +121,10 @@ private BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Diag
                var type = boundRHS.Type ?? voidType;
                return new BoundDeconstructionAssignmentOperator(
                            node,
-                            DeconstructionVariablesAsTuple(node, checkedVariables, diagnostics, hasErrors: true),
+                            DeconstructionVariablesAsTuple(node, checkedVariables, resultIsUsed ? diagnostics : null, hasErrors: true),


Is there a test for this case, where boundRHS.Type.IsErrorType() and resultIsUsed == false? #Resolved

Added one. I'll stop by to discuss. #Resolved

cston · 2017-06-20T22:25:38Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

+        }
+
+        [Fact]
+        public void ValueTupleNotRequiredIfReturnIsUsed2()


...IfReturnIsNotUsed2 #Resolved

jcouv · 2017-06-20T22:26:30Z

src/Compilers/CSharp/Portable/BoundTree/BoundNodes.xml

+  </Node>
+
+  <Node Name="BoundVoid" Base="BoundExpression">
+    <!-- Non-null type is required for this node kind, but it will always be the void type  -->


I'll remove the comment. We'll be using a type that is not "void".
@cston I think this reinforces your discomfort with calling this node BoundVoid. I'll try to think of another name or approach. #Resolved

cston · 2017-06-20T22:26:48Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

@@ -1095,6 +1090,119 @@ public void Deconstruct(out int a, out int b)
        }

        [Fact]
+        public void ValueTupleNotRequiredIfReturnIsNotUsed()


Consider adding [WorkItem(18629)]. #Resolved

cston · 2017-06-20T23:36:36Z

LGTM

AlekseyTs · 2017-06-21T18:05:41Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

@@ -77,6 +77,8 @@ private BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Diag
        /// <param name="diagnostics">Where to report diagnostics</param>
        /// <param name="declaration">A variable set to the first variable declaration found in the left</param>
        /// <param name="expression">A variable set to the first expression in the left that isn't a declaration or discard</param>
+        /// <param name="resultIsUsedOverride">The expression evaluator needs bind deconstructions (both assignments and declarations) as expression-statements


needs bind [](start = 72, length = 10)

needs to bind? #Closed

AlekseyTs · 2017-06-21T18:09:51Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

+        private static bool IsDeconstructionResultUsed(ExpressionSyntax left)
+        {
+            var parent = left.Parent;
+            if (parent is null || parent.Kind() == SyntaxKind.ForEachVariableStatement)


parent is null [](start = 16, length = 14)

Are we building with compiler that performs efficient code gen for this condition? #Closed

Not yet. The master branch is using 2.3.0-beta2-61716-09 (from May 16th), but this optimization went into on May 30th. We will definitely refresh with RTM drop (if not sooner), and this PR is for 15.6. #Resolved

AlekseyTs · 2017-06-21T18:19:32Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

@@ -116,9 +121,10 @@ private BoundExpression BindDeconstruction(AssignmentExpressionSyntax node, Diag
                var type = boundRHS.Type ?? voidType;
                return new BoundDeconstructionAssignmentOperator(
                            node,
-                            DeconstructionVariablesAsTuple(node, checkedVariables, diagnostics, hasErrors: true),
+                            DeconstructionVariablesAsTuple(node, checkedVariables, resultIsUsed ? diagnostics : null, hasErrors: true),


DeconstructionVariablesAsTuple(node, checkedVariables, resultIsUsed ? diagnostics : null, hasErrors: true) [](start = 28, length = 106)

I am not comfortable with this change. It suppresses all diagnostics that could be reported by DeconstructionVariablesAsTuple function. It is very possible that might suppress to much in the future, or even does that already for some scenarios today. I think we should be very precise about what diagnostics we suppress and where. #Closed

AlekseyTs · 2017-06-21T18:27:21Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

+                }
+                else
+                {
+                    returnValue = new BoundUnusedResult(left.Syntax, left.Type);


left.Type [](start = 69, length = 9)

Is this possibly an error type? This doesn't feel right, we probably shouldn't have any references to error types after lowering. #Closed

AlekseyTs · 2017-06-21T18:29:21Z

        return new BoundTupleLiteral(right.Syntax, default(ImmutableArray<string>), default(ImmutableArray<bool>), builder.ToImmutableAndFree(), tupleType);

Why even bother doing all this if we don't need the result? #Closed

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs:111 in 77068fa. [](commit_id = 77068fae0aa9fed6a55cedf0ac8a9f7358152e7a, deletion_comment = False)

AlekseyTs · 2017-06-21T18:31:44Z

src/Compilers/CSharp/Portable/Symbols/Tuples/TupleTypeSymbol.cs

@@ -109,7 +109,7 @@ internal sealed class TupleTypeSymbol : WrappedNamedTypeSymbol
            }

            var constructedType = Create(underlyingType, elementNames, errorPositions, locationOpt, elementLocations);
-            if (shouldCheckConstraints)
+            if (shouldCheckConstraints && diagnostics != null)


&& diagnostics != null [](start = 38, length = 23)

This doesn't feel right, we might be suppressing errors that have nothing to do with availability of the type. #Closed

I guess it is fine to suppress constraints checks as well when the type is not going to be used at the end.

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

AlekseyTs · 2017-06-21T18:55:59Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

@@ -1095,6 +1090,122 @@ public void Deconstruct(out int a, out int b)
        }

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


ValueTupleNotRequiredIfReturnIsNotUsed [](start = 20, length = 38)

Should probably have tests to verify that obsolete diagnostics is suppressed even when the type is present. #Closed

Same for constraint violations.

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

I see that we already have tests covering constraints.

In reply to: 123337083 [](ancestors = 123337083,123336564)

AlekseyTs · 2017-06-21T18:59:51Z

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenDeconstructTests.cs

@@ -1228,21 +1339,6 @@ public void Deconstruct(out ArgIterator a, out int b)
                // (14,46): error CS0306: The type 'ArgIterator' may not be used as a type argument
                //     public static (ArgIterator, ArgIterator) M2()
                Diagnostic(ErrorCode.ERR_BadTypeArgument, "M2").WithArguments("System.ArgIterator").WithLocation(14, 46),
-                // (7,22): error CS0306: The type 'ArgIterator' may not be used as a type argument
-                //         (int x, var (err1, y)) = (0, new C());
-                Diagnostic(ErrorCode.ERR_BadTypeArgument, "err1").WithArguments("System.ArgIterator").WithLocation(7, 22),


Diagnostic(ErrorCode.ERR_BadTypeArgument, "err1").WithArguments("System.ArgIterator").WithLocation(7, 22), [](start = 16, length = 106)

Do we have other tests covering this diagnostics? #Closed

AlekseyTs · 2017-06-21T19:04:30Z

Done with review pass. #Closed

jcouv · 2017-06-22T16:04:05Z

@cston from discussion with aleksey, I changed to use the last element of the tuple return value as the return value. #Resolved

deconstruction

AlekseyTs · 2017-06-27T18:44:14Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

            {
-                returnTuple = VisitExpression(returnTuple);
+                // When a deconstruction is not used, we use the last effect is used as return value


we use the last effect is used as return value [](start = 54, length = 46)

Looks like there is a typo "we use the last effect is used as return value" #Closed

AlekseyTs · 2017-06-27T18:47:05Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

+            }
+            else
+            {
+                if (!returnValue.HasErrors && isUsed)


&& isUsed [](start = 42, length = 10)

It looks like the check for isUsed is redundant, the outer if checked it already. #Closed

AlekseyTs · 2017-06-27T18:48:57Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

+                if (last is null)
+                {
+                    // Deconstructions with no effects lower to nothing. For example, `(_, _) = (1, 2);`
+                    result = null;


result = null; [](start = 20, length = 14)

Are we leaking temps and effects when we take this code path? #Closed

Yes :-( Thanks

AlekseyTs · 2017-06-27T18:50:32Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

            {
-                returnTuple = VisitExpression(returnTuple);
+                // When a deconstruction is not used, we use the last effect is used as return value
+                var last = effects.PopLast();


var last = effects.PopLast(); [](start = 16, length = 29)

Consider asserting that returnValue is null. #Closed

jcouv · 2017-06-27T20:00:37Z

@cston Could you take another look?
The new design takes the last element in the sequence as the return value. If there are no effect, then the whole deconstruction is lowered to null.

cston · 2017-06-27T20:11:49Z

src/Compilers/CSharp/Portable/Binder/Binder_Deconstruct.cs

@@ -467,7 +504,8 @@ private static TypeSymbol MakeMergedTupleType(ArrayBuilder<DeconstructionVariabl
                errorPositions: default(ImmutableArray<bool>));
        }

-        private BoundTupleLiteral DeconstructionVariablesAsTuple(CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables, DiagnosticBag diagnostics, bool hasErrors)
+        private BoundTupleLiteral DeconstructionVariablesAsTuple(CSharpSyntaxNode syntax, ArrayBuilder<DeconstructionVariable> variables,
+            DiagnosticBag diagnostics, bool ignoreDiagnosticsFromTuple)


Is ignoreDiagnosticsFromTuple needed or could the caller pass diagnostics: null in that case?

We could pass null diagnostics, but the feedback from Aleksey was that pushing that logic down (being more explicit) is safer as the code changes in the future.

cston · 2017-06-27T21:04:32Z

...ers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_DeconstructionAssignmentOperator.cs

+                    return last;
+                }
+                return null;
+            }


There are five separate cases in PopLast. Are we testing all of these? Alternatively, would this be simpler if ToImmutableAndFree() is split into separate GetExpressions and Free methods, and the caller handles getting the last expression?

void GetExpressions(ArrayBuilder<Expression> builder) { builder.AddRange(init); builder.AddRange(deconstructions); builder.AddRange(conversions); builder.AddRange(assignments); }

In the caller:

var builder = ArrayBuilder<Expression>.GetInstance(); effects.GetExpressions(builder); effects.Free(); if (isUsed) { result = _factory._factory.Sequence(temps.ToImmutableAndFree(), builder.ToImmutable(), returnValue); } else if (builder.Count == 0) { result = null; } else { var last = builder.Last(); builder.RemoveLast(); result = _factory._factory.Sequence(temps.ToImmutableAndFree(), builder.ToImmutable(), last); } builder.Free();

AlekseyTs

LGTM, assuming tests are passing.

jcouv · 2017-06-28T22:45:38Z

@cston Any other feedback?

cston · 2017-06-28T23:00:45Z

No additional feedback other than simplifying DeconstructionSideEffects as we discussed.

cston · 2017-06-29T00:29:44Z

LGTM

jcouv · 2017-06-29T02:17:01Z

VS integration timeout (details)

I'll re-run

17:58:27 Roslyn Error: test timeout exceeded, dumping remaining processes
17:58:31 Dumping conhost 5388 to D:\j\workspace\windows_relea---bb1c6911\conhost-0.dmp ... succeeded
17:58:51 Dumping devenv 7116 to D:\j\workspace\windows_relea---bb1c6911\devenv-1.dmp ... succeeded
17:58:52 Dumping Microsoft.Alm.Shared.Remoting.RemoteContainer.dll 6092 to D:\j\workspace\windows_relea---bb1c6911\Microsoft.Alm.Shared.Remoting.RemoteContainer.dll-2.dmp ... succeeded
17:58:54 Dumping ScriptedSandbox64 5480 to D:\j\workspace\windows_relea---bb1c6911\ScriptedSandbox64-3.dmp ... succeeded
17:58:55 Dumping ScriptedSandbox64 2516 to D:\j\workspace\windows_relea---bb1c6911\ScriptedSandbox64-4.dmp ... succeeded
17:58:57 Dumping xunit.console.x86 6408 to D:\j\workspace\windows_relea---bb1c6911\xunit.console.x86-5.dmp ... succeeded
17:58:58 Command failed to execute: D:\j\workspace\windows_relea---bb1c6911\Binaries\Release\Exes\RunTests\RunTests.exe C:\Users\dotnet-bot\.nuget\packages\xunit.runner.console\2.2.0-beta4-build3444\tools -xml -timeout:50 -procdumppath:c:\SysInternals D:\j\workspace\windows_relea---bb1c6911\Binaries\Release\UnitTests\VisualStudioIntegrationTests\Roslyn.VisualStudio.IntegrationTests.dll
17:58:58 System.Management.Automation.RuntimeException: Command failed to execute: D:\j\workspace\windows_relea---bb1c6911\Binaries\Release\Exes\RunTests\RunTests.exe C:\Users\dotnet-bot\.nuget\packages\xunit.runner.console\2.2.0-beta4-build3444\tools -xml -timeout:50 -procdumppath:c:\SysInternals D:\j\workspace\windows_relea---bb1c6911\Binaries\Release\UnitTests\VisualStudioIntegrationTests\Roslyn.VisualStudio.IntegrationTests.dll
17:58:58 at Exec-Command, D:\j\workspace\windows_relea---bb1c6911\build\scripts\build-utils.ps1: line 73
17:58:58 at Test-XUnit, D:\j\workspace\windows_relea---bb1c6911\build\scripts\build.ps1: line 248
17:58:58 at <ScriptBlock>, D:\j\workspace\windows_relea---bb1c6911\build\scripts\build.ps1: line 387
17:58:58 Build step 'Execute Windows batch command' marked build as failure
17:58:59 [xUnit] [INFO] - Starting to record.
17:58:59 [xUnit] [INFO] - Processing xUnit.Net-v2 (default)
17:58:59 [xUnit] [INFO] - [xUnit.Net-v2 (default)] - 1 test report file(s) were found with the pattern '**/xUnitResults/*.xml' relative to 'D:\j\workspace\windows_relea---bb1c6911' for the testing framework 'xUnit.Net-v2 (default)'.
17:58:59 
[xUnit] [ERROR] - The result file 'D:\j\workspace\windows_relea---bb1c6911\Binaries\Release\UnitTests\VisualStudioIntegrationTests\xUnitResults\Roslyn.VisualStudio.IntegrationTests.dll.xml' for the metric 'xUnit.Net' is empty. The result file has been skipped.

jcouv · 2017-06-29T02:17:23Z

test windows_debug_vs-integration_prtest please

jcouv · 2017-06-29T02:17:32Z

test windows_release_vs-integration_prtest please

jcouv · 2017-06-29T04:24:20Z

test windows_debug_vs-integration_prtest please

jcouv · 2017-06-29T16:27:03Z

test timeout in VS integration. FYI @dotnet/roslyn-infrastructure (details)

I'll re-run

22:41:45 Roslyn Error: test timeout exceeded, dumping remaining processes
22:41:49 Dumping conhost 6904 to D:\j\workspace\windows_debug---1522b458\conhost-0.dmp ... succeeded
22:41:49 Dumping conhost 1320 to D:\j\workspace\windows_debug---1522b458\conhost-1.dmp ... succeeded
22:41:51 Dumping devenv 4876 to D:\j\workspace\windows_debug---1522b458\devenv-2.dmp ... succeeded
22:41:52 Dumping git 6176 to D:\j\workspace\windows_debug---1522b458\git-3.dmp ... succeeded
22:41:54 Dumping Microsoft.Alm.Shared.Remoting.RemoteContainer.dll 5980 to D:\j\workspace\windows_debug---1522b458\Microsoft.Alm.Shared.Remoting.RemoteContainer.dll-4.dmp ... succeeded
22:41:54 Dumping ScriptedSandbox64 7164 to D:\j\workspace\windows_debug---1522b458\ScriptedSandbox64-5.dmp ... succeeded
22:41:54 Dumping WerFault 3684 to D:\j\workspace\windows_debug---1522b458\WerFault-6.dmp ... succeeded
22:41:55 Dumping xunit.console.x86 6156 to D:\j\workspace\windows_debug---1522b458\xunit.console.x86-7.dmp ... succeeded
22:41:55 Command failed to execute: D:\j\workspace\windows_debug---1522b458\Binaries\Debug\Exes\RunTests\RunTests.exe C:\Users\dotnet-bot\.nuget\packages\xunit.runner.console\2.2.0-beta4-build3444\tools -xml -timeout:50 -procdumppath:c:\SysInternals D:\j\workspace\windows_debug---1522b458\Binaries\Debug\UnitTests\VisualStudioIntegrationTests\Roslyn.VisualStudio.IntegrationTests.dll
22:41:55 System.Management.Automation.RuntimeException: Command failed to execute: D:\j\workspace\windows_debug---1522b458\Binaries\Debug\Exes\RunTests\RunTests.exe C:\Users\dotnet-bot\.nuget\packages\xunit.runner.console\2.2.0-beta4-build3444\tools -xml -timeout:50 -procdumppath:c:\SysInternals D:\j\workspace\windows_debug---1522b458\Binaries\Debug\UnitTests\VisualStudioIntegrationTests\Roslyn.VisualStudio.IntegrationTests.dll

jcouv · 2017-06-29T16:27:11Z

test windows_debug_vs-integration_prtest please

jcouv · 2017-06-29T18:23:24Z

test windows_debug_vs-integration_prtest please

jcouv added the Area-Compilers label Jun 17, 2017

jcouv added this to the 15.5 milestone Jun 17, 2017

jcouv self-assigned this Jun 17, 2017

dnfclas added the cla-already-signed label Jun 17, 2017

jcouv added the New Language Feature - Tuples Tuples label Jun 17, 2017

jcouv mentioned this pull request Jun 17, 2017

Surprising System.ValueTuple requirement for deconstruction #18629

Closed

jcouv added the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jun 17, 2017

jcouv force-pushed the decon-notuples branch from db6631d to bda38cb Compare June 17, 2017 21:32

jcouv removed the PR For Personal Review Only The PR doesn’t require anyone other than the developer to review it. label Jun 20, 2017

cston reviewed Jun 20, 2017

View reviewed changes

jcouv commented Jun 20, 2017

View reviewed changes

cston reviewed Jun 20, 2017

View reviewed changes

AlekseyTs reviewed Jun 21, 2017

View reviewed changes

jcouv force-pushed the decon-notuples branch from 9115053 to fac0f9f Compare June 22, 2017 04:39

jcouv added 2 commits June 27, 2017 10:07

Use the last effect as the result of the sequence, when lowering

11f2783

deconstruction

Tweak comment and fix two tests

127f65a

jcouv force-pushed the decon-notuples branch from d2aad99 to 127f65a Compare June 27, 2017 18:11

AlekseyTs reviewed Jun 27, 2017

View reviewed changes

Fix leak of array builders

a694550

cston reviewed Jun 27, 2017

View reviewed changes

AlekseyTs approved these changes Jun 27, 2017

View reviewed changes

Tweak PopLast on deconstruction effects

43cf584

jcouv closed this Jun 28, 2017

jcouv reopened this Jun 28, 2017

dnfclas added the cla-already-signed label Jun 28, 2017

jcouv merged commit 239af49 into dotnet:dev15.6 Jun 29, 2017

jcouv deleted the decon-notuples branch June 29, 2017 19:45

jcouv mentioned this pull request Oct 20, 2017

Deconstruction on tuple literal doesn't emit ref to ValueTuple #22731

Merged

jcouv mentioned this pull request Jul 7, 2018

Official documentation/specification of C# "swap" syntax through tuples? dotnet/csharplang#1656

Closed

Don't require ValueTuple type in deconstruction if return value is not used #20295

Don't require ValueTuple type in deconstruction if return value is not used #20295

Conversation

jcouv commented Jun 17, 2017 • edited Loading

jcouv commented Jun 17, 2017 • edited Loading

jcouv commented Jun 20, 2017 • edited Loading

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Jun 20, 2017 • edited Loading

Choose a reason for hiding this comment

cston Jun 20, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

cston commented Jun 20, 2017

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Jun 21, 2017 • edited Loading

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017

Choose a reason for hiding this comment

AlekseyTs Jun 21, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Jun 21, 2017 • edited Loading

jcouv commented Jun 22, 2017 • edited Loading

AlekseyTs Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv Jun 27, 2017

Choose a reason for hiding this comment

AlekseyTs Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv commented Jun 27, 2017

cston Jun 27, 2017

Choose a reason for hiding this comment

jcouv Jun 27, 2017

Choose a reason for hiding this comment

cston Jun 27, 2017

Choose a reason for hiding this comment

AlekseyTs left a comment

Choose a reason for hiding this comment

jcouv commented Jun 28, 2017

cston commented Jun 28, 2017

cston commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 29, 2017

jcouv commented Jun 17, 2017 •

edited

Loading

jcouv commented Jun 17, 2017 •

edited

Loading

jcouv commented Jun 20, 2017 •

edited

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

jcouv Jun 20, 2017 •

edited

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

jcouv Jun 20, 2017 •

edited

Loading

cston Jun 20, 2017 •

edited by jcouv

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

jcouv Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs commented Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs Jun 21, 2017 •

edited

Loading

AlekseyTs commented Jun 21, 2017 •

edited

Loading

jcouv commented Jun 22, 2017 •

edited

Loading

AlekseyTs Jun 27, 2017 •

edited

Loading

AlekseyTs Jun 27, 2017 •

edited

Loading

AlekseyTs Jun 27, 2017 •

edited

Loading

AlekseyTs Jun 27, 2017 •

edited

Loading