Implement tuple equality #23356

jcouv · 2017-11-22T06:11:15Z

CyrusNajmabadi · 2017-11-24T22:18:45Z

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

@@ -560,6 +560,14 @@ private BoundExpression BindSimpleBinaryOperator(BinaryExpressionSyntax node, Di

            if (hasErrors)
            {
+                if (GetTupleCardinality(left).HasValue &&
+                    GetTupleCardinality(right).HasValue &&
+                    (kind == BinaryOperatorKind.Equal || kind == BinaryOperatorKind.NotEqual))


Should we maybe 'lift' all operators onto tuples, just like we can with nullable? You could then have:

var x = GetVector() var y = GetOtherVector(); var z = x + y;

While it expands the original proposal, it seems like it would be fine. And since you're here anyways... :)

AFAICT, it would be fine. An operator is 'lifted' up to two tuples, if they have the same length, and if the operator is defined for all pairwise matching element types of the tuples.

What do you think? (Honestly, not trying to feature creep. It just seems like it would make sense and would fit in well here).

--

Also, there are operators like < and >. If we did those, i imagine they would have to be 'all' operators. i.e. a < b if all of a's elements are less than the constiuent element in b. That would mean (and it would make sense) that you would have tuples that were neither <, == or > than another tuple. That makes sense to me and feels fine.
#Resolved

CyrusNajmabadi · 2017-11-24T22:21:21Z

Are there any IOperation concerns here? #Resolved

jcouv · 2017-11-27T17:22:15Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            if (coreLeft.Type.IsNullableType())
+            {
+                BoundExpression loweredLeft = EvaluateSideEffectingArgumentToTemp(left, outerEffects, ref outerTemps);
+                leftHasValue = MakeHasValueTemp(loweredLeft, outerTemps, outerEffects); // PROTOTYPE(tuple-equality) should loweredRight be evaluated before left.HasValue?


📝 I'll address or remove comment before merging. #Closed

jcouv · 2017-11-27T17:22:38Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            }
+        }
+
+        // PROTOTYPE(tuple-equality) Clean up


📝 I'll address before merging. #Closed

jcouv · 2017-11-27T17:22:55Z

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

+            if (hasErrors)
+            {
+                ReportBinaryOperatorError(node, diagnostics, node.OperatorToken, left, right, resultKind);
+                //resultOperatorKind &= ~BinaryOperatorKind.TypeMask; // PROTOTYPE(tuple-equality) Not sure what this is for


📝 I'll address before merging. #Closed

jcouv · 2017-11-27T17:24:27Z

@dotnet/roslyn-compiler for review. Thanks

@CyrusNajmabadi Yes, that's on my radar. I'll add minimal IOperation support in this PR (IOperation.None) and propose how to represent properly for next PR. #Resolved

CyrusNajmabadi · 2017-11-27T19:42:43Z

Yes, that's on my radar.

What's that in reference to? Lifting all operators to tuples? Or doing IOperation support? #Resolved

jcouv · 2017-11-27T20:43:36Z

Yes, that's on my radar.

I meant IOperation support. GetOperation should work on t == (1, 2). My thinking is that it should expose two IOperation nodes (for left and right) and then also the operator kind.

I'd probably not expose the details of the comparison (which element-wise binary operators are involved). That would follow the example of IOperation support for deconstructions (which doesn't currently expose which Deconstruct methods or conversions are involved).

Lifting all operators to tuples

LDM preferred to limit support to just == and != for now. #Resolved

CyrusNajmabadi · 2017-11-27T22:29:43Z

LDM preferred to limit support to just == and != for now.

Thanks! But also: :'(

in practice probably not that useful. And, if really necessary, could always be added later. #Resolved

gafter · 2017-11-28T18:25:04Z

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

+using Microsoft.CodeAnalysis.Test.Utilities;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen


namespace [](start = 0, length = 9)

I don't see any tests comparing nullable tuples of different types, e.g. (int, int)? t1 and (long, long)? t2, where conversions are needed to do the tests. Can you please add such tests? Also, it would be nice to have at least one test involving a user-defined conversion in the mix with nullable too. #Closed

Good point.
Some other test ideas:

nullable tuples used with != (to make sure the binary logic around HasValue is correct).

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

Also, please add a test with tuple cardinality 1 (using a long tuple's Rest field).

In reply to: 153588129 [](ancestors = 153588129,153580386)

Are there tests for tuples passed as in? #Resolved

gafter · 2017-11-28T18:29:47Z

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

@@ -560,6 +560,14 @@ private BoundExpression BindSimpleBinaryOperator(BinaryExpressionSyntax node, Di

            if (hasErrors)
            {
+                if (GetTupleCardinality(left).HasValue &&
+                    GetTupleCardinality(right).HasValue &&


This does not appear to implement the specification-required overload resolution rules to determine which built-in operator to use. For example, if the left-hand operand is a tuple, and the right-hand operand is not a tuple but has a conversion to a tuple, we should be using tuple equality. Please add such a test. #Closed

This may require LDM review/design of the intended rules if that isn't simple, which I suspect may be the case.

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

See TestEqualityOfTypeConvertingFromTuple and TestEqualityOfTypeConvertingToTuple
The conversion to tuple doesn't contribute (so the binding errors out).
The conversion from tuple works as before.

I believe that matches what you and I had discussed with Mads. #Resolved

gafter · 2017-11-28T18:30:31Z

        var best = this.BinaryOperatorOverloadResolution(kind, left, right, node, diagnostics, out resultKind, out originalUserDefinedOperators);

I believe this is where we should be determining that we should be using a tuple equality or inequality operator. #Closed

Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs:507 in 2bbd8e4. [](commit_id = 2bbd8e4, deletion_comment = False)

gafter · 2017-11-28T18:34:39Z

Do we have a draft spec for the intended semantics? #Closed

jcouv · 2017-11-28T18:49:04Z

Here's the proposed spec: dotnet/csharplang#967 #Resolved

jcouv · 2017-11-28T18:52:54Z

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

+            CompileAndVerify(comp, expectedOutput: "TrueFalse");
+        }
+
+        // TODO: test (1, nullableTuple) == (1, tuple)


📝 I'll add those before merging. #Closed

jcouv · 2017-11-28T18:54:23Z

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

+        [Fact]
+        void TestSimpleTypelessTupleAndTupleType()
+        {
+            // WORK-IN-PROGRESS: there is a missing conversion from Int32 to Int64 on t1.Item1


Dead comment. I'll remove #Closed

gafter · 2017-11-28T19:54:54Z

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

+    /// <summary>
+    /// A tree of nested binary operators
+    /// </summary>
+    internal abstract class TupleBinaryOperatorInfo


TupleBinaryOperatorInfo [](start = 28, length = 23)

TupleBinaryOperatorInfo [](start = 28, length = 23)

TupleBinaryOperatorInfo [](start = 28, length = 23)

Can these types be defined in BoundNodes.xml? #Closed

They could, but since they don't correspond to any specific syntax, I'm not sure it would be beneficial. I'm following the model used for foreach loops, awaits and deconstructions, which all use "info" objects.

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

gafter · 2017-11-28T20:04:57Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+        private BoundExpression RewriteTupleSingleOperator(TupleBinaryOperatorInfo.Single single,
+            BoundExpression left, BoundExpression right, TypeSymbol boolType)
+        {
+            return _factory.Binary(single.Kind, boolType, left, right, single.MethodSymbolOpt);


_factory [](start = 19, length = 8)

_factory [](start = 19, length = 8)

I suspect that there may be some lowering not occurring here. For example, what happens if the operands are of type decimal? #Closed

Lowering would turn the operator into a method invocation (e.g. when single.MethodSymbolOpt is non-null)

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

Previously, I had a VisitExpression at the top-level (before returning from VisitTupleBinaryOperator). It turns out that is lowering some nodes twice. I've now fixed the lowering strategy to avoid double-lowering.

In reply to: 153608156 [](ancestors = 153608156,153607936)

gafter

Please add some tests as discussed.

AlekseyTs · 2018-03-05T05:29:50Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            //     leftValue = left.GetValueOrDefault(); (or left if !leftNullable)
+            //     rightValue = right.GetValueOrDefault(); (or right if !rightNullable)
+            //     ... logical expression using leftValue and rightValue ...
+            BoundExpression innerSequence = _factory.Sequence(locals: ImmutableArray<LocalSymbol>.Empty, innerEffects.ToImmutableAndFree(), logicalExpression);


innerEffects.ToImmutableAndFree() [](start = 105, length = 33)

Should probably avoid creating a sequence if there are no side-effects #Closed

AlekseyTs · 2018-03-05T05:31:55Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            //          : false/true
+            bool boolValue = operatorKind == BinaryOperatorKind.Equal; // true/false
+            BoundExpression outerSequence =
+                _factory.Sequence(ImmutableArray<LocalSymbol>.Empty, outerEffects.ToImmutableAndFree(),


outerEffects.ToImmutableAndFree() [](start = 69, length = 33)

Should probably avoid creating a sequence if there are no side-effects #Closed

AlekseyTs · 2018-03-05T05:37:05Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            if (isRightNullable)
+            {
+                rightHasValue = MakeNullableHasValue(right.Syntax, right); // no need for local for right.HasValue since used once
+                rightValue = MakeValueOrDefaultTemp(right, initialEffectsAndTemps.temps, innerEffects);


rightValue = MakeValueOrDefaultTemp(right, initialEffectsAndTemps.temps, innerEffects); [](start = 16, length = 87)

It looks like rightValue is used only once, why do we store it in a temp? #Closed

AlekseyTs · 2018-03-05T05:37:55Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            if (isLeftNullable)
+            {
+                leftHasValue = MakeHasValueTemp(left, initialEffectsAndTemps.temps, outerEffects);
+                leftValue = MakeValueOrDefaultTemp(left, initialEffectsAndTemps.temps, innerEffects);


leftValue = MakeValueOrDefaultTemp(left, initialEffectsAndTemps.temps, innerEffects); [](start = 16, length = 85)

It looks like leftValue is used only once, why do we store it in a temp? #Closed

AlekseyTs · 2018-03-05T05:49:41Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+        /// For tuple literals, we just return the element.
+        /// For expressions with tuple type, we access `Item{i}`.
+        /// </summary>
+        private BoundExpression GetTuplePart(BoundExpression tuple, int i, ArrayBuilder<LocalSymbol> temps, ArrayBuilder<BoundExpression> initEffects)


, ArrayBuilder temps, ArrayBuilder initEffects [](start = 73, length = 76)

It looks like these two parameters are not used. #Closed

AlekseyTs · 2018-03-05T05:57:36Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+                    result = _factory.Not(result);
+                }
+            }
+            else if (boolConversion != Conversion.Identity)


boolConversion != Conversion.Identity [](start = 21, length = 37)

!boolConversion.IsIdentity? #Closed

AlekseyTs · 2018-03-05T06:04:23Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            return result;
+        }
+
+        private class TupleOperatorSideEffectsAndTemps


TupleOperatorSideEffectsAndTemps [](start = 22, length = 32)

With the current approach, it feels like this type does not give us any advantage. Side-effects are only collected locally in VisitTupleBinaryOperator and, I think, the code would be much cleaner if other helpers would keep track of their temps and create sequences accordingly. Even, if we don't change temp management, it is still enough to pass a the same builder for side-effects to ReplaceTerminalElementsWithTemps, and simply pass a builder for temps to other helpers. #Closed

AlekseyTs · 2018-03-05T06:08:37Z

Done with review pass (iteration 17) #Closed

AlekseyTs · 2018-03-05T17:47:20Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            // (1, 2) == (1, 2);
+            if (IsTupleExpression(tuple.Kind))
+            {
+                return ((BoundTupleExpression)tuple).Arguments[i];


return ((BoundTupleExpression)tuple).Arguments[i]; [](start = 16, length = 50)

Can this be anything, but a tuple literal? #Closed

The implementation of IsTupleExpression is return kind == BoundKind.TupleLiteral || kind == BoundKind.ConvertedTupleLiteral; and the two sub-types of BoundTupleExpression are BoundTupleLiteral and BoundConvertedTupleLiteral.

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

The implementation of IsTupleExpression is return kind == BoundKind.TupleLiteral || kind == BoundKind.ConvertedTupleLiteral; and the two sub-types of BoundTupleExpression are BoundTupleLiteral and BoundConvertedTupleLiteral.

Given the behavior of ReplaceTerminalElementsWithTemps, it feels like we cannot get here for anything, but a tuple literal

In reply to: 172275316 [](ancestors = 172275316,172270998)

AlekseyTs · 2018-03-05T20:29:52Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+        {
+            var boolType = node.Type; // we can re-use the bool type
+            var leftInit = ArrayBuilder<BoundExpression>.GetInstance();
+            var rightInit = ArrayBuilder<BoundExpression>.GetInstance();


var rightInit = ArrayBuilder.GetInstance(); [](start = 12, length = 60)

It doesn't look like there is a need to have two builders #Closed

AlekseyTs · 2018-03-05T20:35:52Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+        {
+            // Example:
+            // (1, 2) == (1, 2);
+            if (IsTupleExpression(tuple.Kind))


IsTupleExpression(tuple.Kind) [](start = 16, length = 29)

I would change this condition to cover only tuple literals too. #Closed

AlekseyTs · 2018-03-05T20:39:24Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+
+            // PROTOTYPE(tuple-equality) checked
+            // We leave the null literal in nullable-null conversions unconverted because MakeBinaryOperator has special rules for it
+            bool isNullableNullConversion = ((operatorKind & BinaryOperatorKind.NullableNull) == 0);


bool isNullableNullConversion = ((operatorKind & BinaryOperatorKind.NullableNull) == 0); [](start = 12, length = 88)

Is this logic correct? Should we be checking for == 0 here? Consider to use OperandTypes helper here as well. #Closed

AlekseyTs · 2018-03-05T20:45:10Z

Done with review pass (iteration 18) #Closed

gafter · 2018-03-05T21:33:58Z

        // SPEC: For an operation of one of the forms x == null, null == x, x != null, null != x,

This comment is misplaced. I should be after the tuple business. #Resolved

Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Operators.cs:490 in d9b3a95. [](commit_id = d9b3a95, deletion_comment = False)

AlekseyTs · 2018-03-05T21:47:23Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+
+            // PROTOTYPE(tuple-equality) checked
+            // We leave the null literal in nullable-null conversions unconverted because MakeBinaryOperator has special rules for it
+            bool isNullableNullConversion = operatorKind.OperandTypes() != BinaryOperatorKind.NullableNull;


isNullableNullConversion [](start = 17, length = 24)

There is still something strange about this statement. The local is named "isNullableNullConversion", but it looks like the initializer checks for an opposite situation. Perhaps the local name is confusing, but the comment above also talks about nullable-null conversions. #Closed

Sorry about that. There is definitely something wrong. Investigating.

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

Thanks a lot for catching that, Aleksey.
The element-wise binary operator for nullable-null conversion has a kind, but no operator and no converted types. So no conversion is needed for either side (the null or the other one) during lowering.
There was a similar bug for nullable-null during binding, where I'd used an error type, when in fact leaving the types as null is perfectly fine.

AlekseyTs · 2018-03-05T21:48:23Z

Done with review pass (iteration 19) #Closed

gafter · 2018-03-05T21:54:26Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+        /// - nested expressions that aren't tuple literals, like `GetTuple()` in `(..., GetTuple()) == (..., (..., ...))`
+        /// On the other hand, `Item1` and `Item2` of `GetTuple()` are not saved as part of the initialization phase of `GetTuple() == (..., ...)`
+        ///
+        /// Element-wise conversions occur late, together with the element-wise comparisons. They may not be evaluated.


may [](start = 98, length = 3)

"may" should be "might". "may not" is an idiom forbidding something. #Resolved

gafter · 2018-03-05T22:03:52Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+            // Examples:
+            // in `expr == (..., ...)` we need to save `expr` because it's not a tuple literal
+            // in `(..., expr) == (..., (..., ...))` we need to save `expr` because it is used in a simple comparison
+            return EvaluateSideEffectingArgumentToTemp(VisitExpression(expr), initEffects, ref temps);


EvaluateSideEffectingArgumentToTemp [](start = 19, length = 35)

In looking at the (existing) implementation of the method EvaluateSideEffectingArgumentToTemp, I don't believe it will spill this in a value type. I don't know if that was right or wrong before, but that would appear to be incorrect for this use. #Resolved

Good catch. Added test TestThisStruct with PROTOTYPE marker

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

gafter · 2018-03-05T22:07:08Z

src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_TupleBinaryOperator.cs

+                BoundExpression dynamicResult = _dynamicFactory.MakeDynamicBinaryOperator(single.Kind, left, right, isCompoundAssignment: false, _compilation.DynamicType).ToExpression();
+                if (operatorKind == BinaryOperatorKind.Equal)
+                {
+                    return _factory.Not(MakeUnaryOperator(UnaryOperatorKind.DynamicFalse, left.Syntax, method: null, dynamicResult, boolType));


_factory.Not(MakeUnaryOperator(UnaryOperatorKind.DynamicFalse [](start = 27, length = 61)

This looks like a double negative. #Resolved

I see, the different operator adds a third negation to make them as different as they are supposed to be.

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

gafter

AlekseyTs

LGTM (Iteration 21)

jcouv · 2018-03-06T21:37:28Z

Thanks a bunch 🎉
I'll merge this and get started on restoring semantic model behavior and basic IOperation (as two separate PRs).

jcouv added the Area-Compilers label Nov 22, 2017

jcouv added this to the 15.7 milestone Nov 22, 2017

jcouv self-assigned this Nov 22, 2017

jcouv changed the title ~~Allow comparing tuple types~~ Allow comparing tuples Nov 22, 2017

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

jcouv force-pushed the tuple-equality branch from c951257 to 9c79e1e Compare November 22, 2017 19:58

CyrusNajmabadi reviewed Nov 24, 2017

View reviewed changes

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

jcouv commented Nov 27, 2017

View reviewed changes

gafter reviewed Nov 28, 2017

View reviewed changes

jcouv commented Nov 28, 2017

View reviewed changes

gafter reviewed Nov 28, 2017

View reviewed changes

gafter previously requested changes Nov 28, 2017

View reviewed changes

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

jcouv requested a review from a team as a code owner January 23, 2018 00:09

jcouv force-pushed the tuple-equality branch 2 times, most recently from c426be8 to b0cf72a Compare January 30, 2018 19:11

AlekseyTs reviewed Mar 5, 2018

View reviewed changes

jcouv mentioned this pull request Mar 5, 2018

Incorrect check on BinaryOperatorKind.NullableNull #25231

Closed

Address PR feedback from Aleksey

032159d

AlekseyTs reviewed Mar 5, 2018

View reviewed changes

Use single builder for effects. Strengthen op kind logic

d9b3a95

AlekseyTs reviewed Mar 5, 2018

View reviewed changes

gafter reviewed Mar 5, 2018

View reviewed changes

Address PR feedback from Neal

d96dcfc

gafter approved these changes Mar 6, 2018

View reviewed changes

Fix issue with nullable-null lowering

364bbc9

jcouv force-pushed the tuple-equality branch from 46915b5 to 364bbc9 Compare March 6, 2018 04:57

AlekseyTs approved these changes Mar 6, 2018

View reviewed changes

jcouv merged commit 6c6faaf into dotnet:features/tuple-equality Mar 6, 2018

jcouv mentioned this pull request Mar 21, 2018

Tuple equality: Optimize cases where nullable always has value #25644

Merged

Implement tuple equality #23356

Implement tuple equality #23356

Conversation

jcouv commented Nov 22, 2017

CyrusNajmabadi Nov 24, 2017 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi commented Nov 24, 2017 • edited by jcouv Loading

jcouv Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv Nov 27, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv commented Nov 27, 2017 • edited Loading

CyrusNajmabadi commented Nov 27, 2017 • edited by jcouv Loading

jcouv commented Nov 27, 2017 • edited Loading

CyrusNajmabadi commented Nov 27, 2017 • edited by jcouv Loading

gafter Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cston Feb 13, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

gafter Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcouv Nov 28, 2017 • edited by gafter Loading

Choose a reason for hiding this comment

gafter commented Nov 28, 2017 • edited Loading

gafter commented Nov 28, 2017 • edited Loading

jcouv commented Nov 28, 2017 • edited Loading

jcouv Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

jcouv Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

gafter Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gafter Nov 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gafter left a comment

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Mar 5, 2018 • edited Loading

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Mar 5, 2018 • edited Loading

gafter commented Mar 5, 2018 • edited by jcouv Loading

AlekseyTs Mar 5, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs commented Mar 5, 2018 • edited Loading

gafter Mar 5, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

gafter Mar 5, 2018 • edited by jcouv Loading

Choose a reason for hiding this comment

CyrusNajmabadi Nov 24, 2017 •

edited by jcouv

Loading

CyrusNajmabadi commented Nov 24, 2017 •

edited by jcouv

Loading

jcouv Nov 27, 2017 •

edited

Loading

jcouv Nov 27, 2017 •

edited

Loading

jcouv Nov 27, 2017 •

edited

Loading

jcouv commented Nov 27, 2017 •

edited

Loading

CyrusNajmabadi commented Nov 27, 2017 •

edited by jcouv

Loading

jcouv commented Nov 27, 2017 •

edited

Loading

CyrusNajmabadi commented Nov 27, 2017 •

edited by jcouv

Loading

gafter Nov 28, 2017 •

edited

Loading

cston Feb 13, 2018 •

edited by jcouv

Loading

gafter Nov 28, 2017 •

edited

Loading

jcouv Nov 28, 2017 •

edited by gafter

Loading

gafter commented Nov 28, 2017 •

edited

Loading

gafter commented Nov 28, 2017 •

edited

Loading

jcouv commented Nov 28, 2017 •

edited

Loading

jcouv Nov 28, 2017 •

edited

Loading

jcouv Nov 28, 2017 •

edited

Loading

gafter Nov 28, 2017 •

edited

Loading

gafter Nov 28, 2017 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs commented Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs commented Mar 5, 2018 •

edited

Loading

gafter commented Mar 5, 2018 •

edited by jcouv

Loading

AlekseyTs Mar 5, 2018 •

edited

Loading

AlekseyTs commented Mar 5, 2018 •

edited

Loading

gafter Mar 5, 2018 •

edited by jcouv

Loading

gafter Mar 5, 2018 •

edited by jcouv

Loading

gafter Mar 5, 2018 •

edited

Loading