Nullable bug fixes #34891

jaredpar · 2019-04-09T23:12:55Z

This comprises the following changes:

Renames SetUnknownNullability* to SetObliviousNullability*
Fixes a stack overflow in SetObliviousNullabilityForReferenceTypes (StackOverflowException in SetUnknownNullabilityForReferenceTypes. #30023).
Completes the duplication of Follow up on comments in ImplicitConversions_07 unit-test #29898 to Update methods from initial binding based on inferred nullability #29605 by updating a comment

jaredpar · 2019-04-09T23:13:15Z

@dotnet/roslyn-compiler for review

gafter · 2019-04-09T23:31:24Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                    elementType = array.ElementTypeWithAnnotations;
+                    array = elementType.TypeKind == TypeKind.Array
+                        ? (ArrayTypeSymbol)elementType.Type
+                        : null;


I don't believe this is faster than array = elementType.Type as ArrayTypeSymbol;
In fact I bet it is slower. #Resolved

gafter · 2019-04-09T23:33:19Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                // The language supports deeply nested arrays, up to 10,000+ instances. This means we can't implemented a head
+                // recursive solution here. Need to take an iterative approach to building up the annotations here.
+                var builder = ArrayBuilder<ArrayTypeSymbol>.GetInstance();
+


This is good, but a stack would be clearer, if you were motivated to add a pooled stack. #Resolved

Good point. See so many ArrayBuilder I sometimes forget that we have stacks 😉 #Resolved

Consider using ArrayBuilderExtensions.Push() and Pop() instead.

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

Agreed. Would prefer a stack here, it would make the second part of this much easier to reason about. #Resolved

gafter · 2019-04-09T23:36:48Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                builder.Free();
+                return array;
+            }
+            return WithElementType(ElementTypeWithAnnotations.SetObliviousNullabilityForReferenceTypes());


return [](start = 12, length = 6)

return [](start = 12, length = 6)

nit: newline above. #Resolved

gafter

gafter · 2019-04-09T23:39:02Z

I wanted to call my PR "Nullable bug fixes". Now I have to think up a new name.

cston · 2019-04-10T00:04:25Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                builder.Free();
+                return array;
+            }
+            return WithElementType(ElementTypeWithAnnotations.SetObliviousNullabilityForReferenceTypes());


WithElementType [](start = 19, length = 15)

WithElementType [](start = 19, length = 15)

Is this different than the base case in the iterative approach above? In short, do we need to differentiate the case where the top-level element type is not an array? #Resolved

cston · 2019-04-10T00:17:35Z

src/Compilers/CSharp/Test/Symbol/Symbols/TypeTests.cs

@@ -36,8 +36,7 @@ public void Bug18280()
            var arr = x.Type;

            arr.GetHashCode();
-            // https://github.com/dotnet/roslyn/issues/30023: StackOverflowException in SetUnknownNullabilityForReferenceTypes.
-            //arr.SetUnknownNullabilityForReferenceTypes();
+            arr.SetObliviousNullabilityForReferenceTypes();


arr [](start = 12, length = 3)

arr [](start = 12, length = 3)

Consider checking nullability on each element type, before and after call. #Resolved

333fred · 2019-04-10T06:03:00Z

I wanted to call my PR "Nullable bug fixes". Now I have to think up a new name.

@gafter "Nullabler nullable bug fixes"

jaredpar · 2019-04-10T15:38:12Z

@gafter, @cston thanks, updated based on your feedback.

cston · 2019-04-10T15:40:05Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

-                }
+            // Fixup the element type on the most nested array
+            elementType = elementType.SetObliviousNullabilityForReferenceTypes();
+            array = builder.Pop().WithElementType(elementType);


Is this Pop() case necessary of would it be handled by the while loop below? #Resolved

Yes it's necessary. The first WithElementType is a special case because it needs to be the result of TypeWithAnnotations.SetObliviousNullabilityFoReferenceTypes. This specific type is opaque here, or not array, we don't special case it in any way. The rest of the WithElementType calls are operating on ArrayTypeSymbol instances and they have to manually re-create the TypeWithAnnotations
#Resolved

Can the fixup be written as:

elementType = elementType.SetObliviousNullabilityForReferenceTypes(); while (builder.Count > 0) { array = builder.Pop().WithElementType(elementType); elementType = TypeWithAnnotations.Create(array, NullableAnnotation.Oblivious, array.ElementTypeWithAnnotations.CustomModifiers); }

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

Pretty sure that messes up the custom modifiers. Let's say that arrayA has element type arrayB. In this sample the first Pop returns arrayB and sets elementType to have the custom modifiers of the original elementType (outside the loop). What it really needs though is to take the custom modifiers off of the element on arrayA.

In reply to: 274056977 [](ancestors = 274056977,274030114)

cston · 2019-04-10T15:44:34Z

src/Compilers/CSharp/Test/Symbol/Symbols/TypeTests.cs

+                do
+                {
+                    var next = arrayType.ElementType as ArrayTypeSymbol;
+                    if (next is object)


Is this check needed? #Resolved

Yes. The final element type is int and this method is iterating over arrays. #Resolved

Ah I missed that. In that case, consider having the Action check x.NullableAnnotation rather than x.Element.NullableAnnotation. #Resolved

cston · 2019-04-10T15:53:11Z

src/Compilers/CSharp/Test/Symbol/Symbols/TypeTests.cs

+            type.GetHashCode();
+            foreachArray(x => Assert.Equal(NullableAnnotation.NotAnnotated, x.ElementTypeWithAnnotations.NullableAnnotation));
+            type = type.SetObliviousNullabilityForReferenceTypes();
+            foreachArray(x => Assert.Equal(NullableAnnotation.Oblivious, x.ElementTypeWithAnnotations.NullableAnnotation));


Minor point: It might be simpler to pass in the expected NullableAnnotation to the local function rather than an Action. #Resolved

AlekseyTs · 2019-04-10T17:28:00Z

src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs

-        {
-            return SetNullabilityForReferenceTypes(s_setUnknownNullability);
-        }
+        internal abstract TypeSymbol SetObliviousNullabilityForReferenceTypes();

        private readonly static Func<TypeWithAnnotations, TypeWithAnnotations> s_setUnknownNullability =


s_setUnknownNullability [](start = 79, length = 23)

s_setUnknownNullability [](start = 79, length = 23)

Is this delegate used anywhere? #Closed

It can be deleted. #Resolved

AlekseyTs · 2019-04-10T17:36:44Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

        {
-            return WithElementType(transform(ElementTypeWithAnnotations));
+            // The language supports deeply nested arrays, up to 10,000+ instances. This means we can't implemented a head
+            // recursive solution here. Need to take an iterative approach to building up the annotations here.


Need to take an iterative approach to building up the annotations here. [](start = 40, length = 71)

Why are we concerned with a recursion only within arrays, what about a recursion for pointer types or generic types? Also, it looks like we are fine with relying on recursion for AddNullableTransforms, ApplyNullableTransforms and MergeNullability. Why SetObliviousNullabilityForReferenceTypes is more important? Are we addressing a real problem? #Closed

Why SetObliviousNullabilityForReferenceTypes is more important?

The bug filed was for this case. If we want to expand out to the other cases that's fine to.

what about a recursion for pointer types or generic types?

It's the level of nesting that is the concern. The compiler supports nesting levels of ~500 for generics. That's a reasonable depth for recursive solutions. The tests for this array case indicate we support arrays at a depth of 10,000+ which is not suitable for recursion (blows the stack every time). There are no tests I'm aware of for pointers that suggest we support that level of nesting.

#Resolved

The bug filed was for this case. If we want to expand out to the other cases that's fine to.

I think that in this case we should be concerned about any API that takes recursive approach over arrays. If we don't have a targeted test, that doesn't mean we don't have a problem. I personally not convinced that we have to handle 10000+ deep arrays.

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

Here is the original customer bug for the test case. They do indeed have arrays of incredible depth in their code base (see Class1pf3fbcd0.nxt.cs in the attachment for an example). It's to validate their serialization framework (from the look of it).

http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/531552 #Resolved

Here is the original customer bug for the test case. They do indeed have arrays of incredible depth in their code base (see Class1pf3fbcd0.nxt.cs in the attachment for an example). It's to validate their serialization framework (from the look of it).

There are always people that push things to the extreme. If we think we absolutely have to support them, then I think we should be concerned with AddNullableTransforms, ApplyNullableTransforms and MergeNullability as well.

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

AlekseyTs

LGTM (iteration 7)

I changed a bunch

jaredpar · 2019-04-11T18:26:30Z

The PR is now updated to make ApplyNullableTransforms, AddNullableTransforms, Equals and MergeNullability all iterative.

Note: happy to help explain the style of transform I added to ArrayTypeSymbol here. This type of transform, bears a resemblance to CSP, is necessary in order to make this process iterative. Don't believe there is a good way to simplify this without lots of duplication. If this makes us re-evaluate if we want to support this scenario then that might be a good discussion to have.

@cston, @AlekseyTs, @gafter, @333fred I dismissed your reviews as this was a substantial change to the PR and warrants a re-review.

gafter · 2019-04-11T19:36:54Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

-            if ((object)other == null || !other.HasSameShapeAs(this) ||
-                !other.ElementTypeWithAnnotations.Equals(ElementTypeWithAnnotations, comparison))
+            // We don't want to blow the stack if we have a type like T[][][][][][][][]....[][],
+            // so we do not recurse until we have a non-array. Rather, hash all the ranks together


hash [](start = 71, length = 4)

Please adapt this comment to Equals. Perhaps something like

// We don't want to blow the stack if we have a type like T[][][][][][][][]....[][], // so we do not recurse until we have a non-array. Rather, check all the ranks // and then check the final "T" type. ``` #Resolved

gafter · 2019-04-11T19:40:32Z

src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs

+    /// Represents a sequence of <see cref="NullableAnnotation"/> to apply to a type. The sequence of
+    /// flags is specific to how the individual <see cref="TypeSymbol"/> walk their nested types.
+    /// </summary>
+    internal sealed class NullableTransformStream


NullableTransformStream [](start = 26, length = 23)

Perhaps move to its own source file? #Resolved

gafter · 2019-04-11T19:42:10Z

src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs

+    {
+        private static readonly ObjectPool<NullableTransformStream> Pool = new ObjectPool<NullableTransformStream>(() => new NullableTransformStream(), Environment.ProcessorCount);
+
+        private ImmutableArray<byte> _transforms;


private [](start = 8, length = 7)

readonly #WontFix

This can be mutated as it's a pooled object.

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

gafter · 2019-04-11T19:45:22Z

src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs

+        }
+
+        public static NullableTransformStream Create(byte defaultTransform)
+        {


/// <summary>Returns an infinite stream of the given transform value.</summary> #Resolved

gafter · 2019-04-12T19:03:27Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

            }
+            while (true);
+


Simpler, I think, to use a for loop:

for (var array = this; array != null; array = array.ElementType as ArrayTypeSymbol) { builder.Push((array, stream.GetNextTransform())); }

#Resolved

Or like this:

var array = this; do { builder.Push((array, stream.GetNextTransform())); array = array.ElementType as ArrayTypeSymbol; } while (!(array is null));

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

gafter · 2019-04-12T19:08:30Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                lastState.array.ElementTypeWithAnnotations.Type.ApplyNullableTransforms(stream),
+                lastState.transform);
+            var finalArray = lastState.array.WithElementType(lastElementType);
+


I suspect this would be simpler if you started with the final element type (0 case) rather than the final array (1 case).

var finalType = builder.Peek().array.ElementTypeWithAnnotations.Type.ApplyNullableTransforms(stream); while (builder.Count > 0) { var state = builder.Pop(); var elementType = apply(state.array.ElementTypeWithAnnotations, finalType, state.transform); finalType = state.array.WithElementType(elementType); } builder.Free(); return finalType;

after which apply can be inlined.

gafter · 2019-04-12T19:09:16Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+
+            // First build up the state for each of the ArrayTypeSymbol in the chain of arrays.
+            var array = this;
+            builder.Push(array);


builder [](start = 12, length = 7)

Similar suggestions here. #Resolved

gafter · 2019-04-12T19:09:47Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+
+            // First build up the pair of arrays that need to be processed.
+            var state = (thisArray: this, otherArray: (ArrayTypeSymbol)other);
+            builder.Push(state);


Similarly here. #Resolved

gafter · 2019-04-12T19:21:49Z

Personally I much prefer the latter iteration (custom iterative algorithm), particularly if simplified as suggested.

gafter

AlekseyTs · 2019-04-12T20:10:54Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+            var finalArray = lastState.array.WithElementType(lastElementType);
+
+            // Unwind the stack and rebuild the array chain.
+            while (builder.Count > 0)


while (builder.Count > 0) [](start = 11, length = 26)

Consider the following refactoring:

TypeSymbol result = builder.Peek().array.ElementTypeWithAnnotations.Type.ApplyNullableTransforms(stream); do { var state = builder.Pop(); var twa = TypeWithAnnotations.Create(result, state.array.ElementTypeWithAnnotations.NullableAnnotation, state.array.ElementTypeWithAnnotations.CustomModifiers); if (state.transform.HasValue && twa.ApplyNullableTransformShallow(state.transform.GetValueOrDefault()) is TypeWithAnnotations other) { twa = other; } else { stream.SetHasInsufficientData(); } result = state.array.WithElementType(twa); } while (builder.Count != 0); return result; ``` #Closed

AlekseyTs · 2019-04-12T20:22:16Z

Done with review pass (iteration 16) #Closed

jaredpar · 2019-04-12T21:06:30Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                finalArray = state.thisArray.WithElementType(elementType);
+            }
+
+            return finalArray;
        }


Notice I have a missing builder.Free call here. Everyone missed it on the review. I only noticed it as I was applying the refactorings suggested.

This is what worries me about the separate implementations. Subtle issues like this. #ByDesign

jaredpar · 2019-04-12T21:16:43Z

Updated based on latest suggestions.

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

AlekseyTs · 2019-04-12T21:32:44Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+                    state.array.ElementTypeWithAnnotations.NullableAnnotation,
+                    state.array.ElementTypeWithAnnotations.CustomModifiers);
+                if (state.transform.HasValue &&
+                    elementType.ApplyNullableTransformShallow(state.transform.Value) is TypeWithAnnotations other)


Value [](start = 78, length = 5)

Minor: GetVAlueOrDefault()? To save on the null check that we already done #Closed

cston · 2019-04-12T21:34:59Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+            var finalArray = builder.Peek().array.ElementType.ApplyNullableTransforms(stream);
+            do
+            {
+                var state = builder.Pop();


var state [](start = 16, length = 9)

Please consider deconstructing:

var (array, transform) = builder.Pop(); ``` #Closed

Hadn' tthought o f that.

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

AlekseyTs

LGTM (iteration 17)

cston · 2019-04-12T21:40:38Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

-            TypeWithAnnotations newElementType;
+            // The language supports deeply nested arrays, up to 10,000+ instances. This means we can't implemented a head
+            // recursive solution here. Need to take an iterative approach.
+            var builder = ArrayBuilder<(ArrayTypeSymbol array, byte? transform)>.GetInstance();


(ArrayTypeSymbol array, byte? transform) [](start = 39, length = 40)

I thought the latest guidance was to use leading uppercase for tuple element names.

Latest from CoreFX. Our code base hasn't adjusted yet. I'm using the existing style. Will fix it all up in one go once everything is stetled.

cston · 2019-04-12T21:44:39Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+
+            // First build up the pair of arrays that need to be processed.
+            for (
+                (ArrayTypeSymbol array, ArrayTypeSymbol other) it = (this, (ArrayTypeSymbol)other);


(ArrayTypeSymbol array, ArrayTypeSymbol other) [](start = 16, length = 46)

Leading uppercase for tuple element names.

cston · 2019-04-12T21:45:34Z

src/Compilers/CSharp/Portable/Symbols/ArrayTypeSymbol.cs

+            var finalArray = state.thisArray.ElementType.MergeNullability(state.otherArray.ElementType, variance);
+            do
+            {
+                state = builder.Pop();


state [](start = 16, length = 5)

Consider deconstructing. #Closed

cston · 2019-04-12T22:00:15Z

src/Compilers/CSharp/Portable/Symbols/Source/CustomModifierUtils.cs

+            var stream = NullableTransformStream.GetInstance(flagsBuilder.ToImmutableAndFree());
+            var result = resultType.ApplyNullableTransforms(stream);
+            Debug.Assert(stream.IsComplete);
+            resultType = stream.IsComplete ? result : resultType;


r [](start = 12, length = 1)

stream.Free();

cston · 2019-04-12T22:02:26Z

src/Compilers/CSharp/Portable/Symbols/Metadata/PE/NullableTypeDecoder.cs

                return result;
            }

            // No NullableAttribute applied to the target symbol, or flags do not line-up, return unchanged metadataType.
+            stream.Free();
            return metadataType;


Consider reordering to free in a single code path.

if (!stream.IsComplete) result = metadataType; stream.Free(); return result;

cston · 2019-04-12T22:07:34Z

src/Compilers/CSharp/Portable/Symbols/NullableTransformStream.cs

+        /// Returns whether or not there is data remaining in the series. When a single flag is used this
+        /// will always return false.
+        /// </summary>
+        public bool HasUnusedTransforms => !IsDefault && _positionOrDefault >= 0 && _positionOrDefault < _transforms.Length;


public [](start = 8, length = 6)

private

cston · 2019-04-12T22:11:18Z

src/Compilers/CSharp/Portable/Symbols/NullableTransformStream.cs

+
+        public void Free()
+        {
+            _transforms = default;


_transforms = default; [](start = 12, length = 22)

Consider resetting _positionOrDefault as well.

RikkiGibson · 2019-06-05T20:43:47Z

Is this PR still active?

jaredpar added the Area-Compilers label Apr 9, 2019

jaredpar requested a review from a team as a code owner April 9, 2019 23:12

gafter reviewed Apr 9, 2019

View reviewed changes

gafter previously approved these changes Apr 9, 2019

View reviewed changes

cston reviewed Apr 10, 2019

View reviewed changes

cston previously approved these changes Apr 10, 2019

View reviewed changes

333fred previously approved these changes Apr 10, 2019

View reviewed changes

AlekseyTs reviewed Apr 10, 2019

View reviewed changes

AlekseyTs previously approved these changes Apr 10, 2019

View reviewed changes

jaredpar force-pushed the fix-quick branch from 0e944f4 to ea89ac3 Compare April 11, 2019 18:22

gafter reviewed Apr 11, 2019

View reviewed changes

jaredpar force-pushed the fix-quick branch from ea89ac3 to 717216e Compare April 12, 2019 01:47

gafter reviewed Apr 12, 2019

View reviewed changes

gafter approved these changes Apr 12, 2019

View reviewed changes

AlekseyTs reviewed Apr 12, 2019

View reviewed changes

jaredpar commented Apr 12, 2019

View reviewed changes

Update iterative approach

75de4f0

AlekseyTs reviewed Apr 12, 2019

View reviewed changes

cston reviewed Apr 12, 2019

View reviewed changes

AlekseyTs approved these changes Apr 12, 2019

View reviewed changes

cston reviewed Apr 12, 2019

View reviewed changes

PR feedback

2788f89

cston reviewed Apr 12, 2019

View reviewed changes

cston approved these changes Apr 12, 2019

View reviewed changes

jcouv added this to the 16.2 milestone Apr 23, 2019

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

jinujoseph modified the milestones: 16.2, Backlog Aug 5, 2019

jaredpar closed this Nov 27, 2019

Nullable bug fixes #34891

Nullable bug fixes #34891

Conversation

jaredpar commented Apr 9, 2019

jaredpar commented Apr 9, 2019

gafter Apr 9, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 9, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

jaredpar Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

333fred Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 9, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter left a comment

Choose a reason for hiding this comment

gafter commented Apr 9, 2019

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

333fred commented Apr 10, 2019 • edited Loading

jaredpar commented Apr 10, 2019

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

jaredpar Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

cston Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

jaredpar Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

cston Apr 10, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

AlekseyTs Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

jaredpar Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

jaredpar Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

AlekseyTs Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

jaredpar Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs left a comment

Choose a reason for hiding this comment

jaredpar commented Apr 11, 2019

gafter Apr 11, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 11, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 11, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gafter Apr 11, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 12, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gafter Apr 12, 2019 • edited Loading

Choose a reason for hiding this comment

gafter Apr 12, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter Apr 12, 2019 • edited by jaredpar Loading

Choose a reason for hiding this comment

gafter commented Apr 12, 2019

gafter left a comment

Choose a reason for hiding this comment

AlekseyTs Apr 12, 2019 • edited Loading

Choose a reason for hiding this comment

AlekseyTs commented Apr 12, 2019 • edited Loading

gafter Apr 9, 2019 •

edited by jaredpar

Loading

gafter Apr 9, 2019 •

edited by jaredpar

Loading

jaredpar Apr 9, 2019 •

edited

Loading

333fred Apr 10, 2019 •

edited by jaredpar

Loading

gafter Apr 9, 2019 •

edited by jaredpar

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

333fred commented Apr 10, 2019 •

edited

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

jaredpar Apr 10, 2019 •

edited

Loading

cston Apr 10, 2019 •

edited

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

jaredpar Apr 10, 2019 •

edited

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

cston Apr 10, 2019 •

edited by jaredpar

Loading

AlekseyTs Apr 10, 2019 •

edited

Loading

jaredpar Apr 10, 2019 •

edited

Loading

AlekseyTs Apr 10, 2019 •

edited

Loading

jaredpar Apr 10, 2019 •

edited

Loading

AlekseyTs Apr 10, 2019 •

edited

Loading

jaredpar Apr 10, 2019 •

edited

Loading

gafter Apr 11, 2019 •

edited by jaredpar

Loading

gafter Apr 11, 2019 •

edited by jaredpar

Loading

gafter Apr 11, 2019 •

edited by jaredpar

Loading

gafter Apr 11, 2019 •

edited by jaredpar

Loading

gafter Apr 12, 2019 •

edited by jaredpar

Loading

gafter Apr 12, 2019 •

edited

Loading

gafter Apr 12, 2019 •

edited by jaredpar

Loading

gafter Apr 12, 2019 •

edited by jaredpar

Loading

AlekseyTs Apr 12, 2019 •

edited

Loading

AlekseyTs commented Apr 12, 2019 •

edited

Loading

jaredpar Apr 12, 2019 •

edited

Loading

AlekseyTs Apr 12, 2019 •

edited

Loading

cston Apr 12, 2019 •

edited

Loading

cston Apr 12, 2019 •

edited

Loading