Change `??=` for nullable value types as specified in LDM by 333fred · Pull Request #36329 · dotnet/roslyn

333fred · 2019-06-11T18:14:57Z

Implement the changes from dotnet/csharplang#2591. a ??= b, where a is int? and b is int now has a type of int instead of int?, and a different lowered form. @cston @dotnet/roslyn-compiler for review.

…ying value type when targetting a nullable value type.

333fred · 2019-06-12T17:10:35Z

Ping @dotnet/roslyn-compiler @cston for a review please. #Closed

jcouv · 2019-06-12T17:27:12Z

Looking #Closed

jcouv · 2019-06-12T17:40:17Z

src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf

-        <target state="translated">Hodnota default se převede na null, nikoli na default({0}).</target>
-        <note />
-      </trans-unit>
      <trans-unit id="WRN_DefaultLiteralConvertedToNullIsNotIntended_Title">


WRN_DefaultLiteralConvertedToNullIsNotIntended_Title [](start = 22, length = 52)

WRN_DefaultLiteralConvertedToNullIsNotIntended_Title can also be removed #Closed

jcouv · 2019-06-12T17:43:05Z

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

-        // This should issue a warning, as we're converting to A, not A0
        int? i2 = null;
-        UseParam(i2 ??= default); // warn
+        UseParamAsInt(i2 ??= default);


default [](start = 29, length = 7)

Please verify semantic model on default. Also, let's verify the resulting value in execution (could be done in a standalone test).
If I understood correctly, this is now default(int) (as opposed to default(int?)/null previously). #Closed

jcouv · 2019-06-12T17:43:59Z

        // must be A (where A is the type of a), as it is where the result of the assignment will end up. Therefore,

is this comment still correct? #Closed

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

jcouv · 2019-06-12T17:49:19Z

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

+            if (leftType.IsNullableType())
            {
-                Error(diagnostics, ErrorCode.WRN_DefaultLiteralConvertedToNullIsNotIntended, node.Right, leftType.GetNullableUnderlyingType());
+                // If B is implicitly convertible to A0, then the result type of this expression is A0, except if B is dynamic.


except if B is dynamic. [](start = 104, length = 23)

Why the exception for dynamic? Seems irregular #ByDesign

This is explicitly called out in the spec, it's the same behavior that ?? has.

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

jcouv · 2019-06-12T17:55:26Z

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

+                    diagnostics.Add(node, useSiteDiagnostics);
+                    var convertedRightOperand = CreateConversion(rightOperand, underlyingRightConversion, underlyingLeftType, diagnostics);
+                    // Create a nullable conversion for use by the local rewriter
+                    var nullableConversion = Conversions.ClassifyImplicitConversionFromType(underlyingLeftType, leftType, ref useSiteDiagnostics);


nullableConversion [](start = 24, length = 18)

This conversion doesn't seem to be used (it's not saved into the bound node we create) #Closed

Whoops, forgot to remove. I was using this at one point and then removed it from the bound node, but forgot to remove the creation.

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

333fred · 2019-06-12T18:03:27Z

        // must be A (where A is the type of a), as it is where the result of the assignment will end up. Therefore,

I've updated a few comments to be more explicit about types.

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

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

jcouv · 2019-06-12T18:07:01Z

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

            LearnFromNullTest(leftOperand, ref this.State);
+            if (node.IsNullableValueTypeAssignment)
+            {
+                targetType = TypeWithAnnotations.Create(node.Type, NullableAnnotation.NotAnnotated);


targetType = TypeWithAnnotations.Create(node.Type, NullableAnnotation.NotAnnotated); [](start = 16, length = 84)

Can we add a nullability test to cover this? ((nullableInt ??= default).ToString() or something along those lines) #Closed

jcouv · 2019-06-12T19:55:20Z

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

+                // if (!lhsRead.HasValue) { tmp = loweredRight; transformedAssignment = tmp; }
+                // tmp
+                //
+                // Except that a is only evaluated once.


Except that a is only evaluated once. [](start = 19, length = 37)

I didn't understand this part. a does not appear in the above lowered form. #Closed

Ignore, I'll remove this line.

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

jcouv · 2019-06-12T19:56:38Z

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

+                // Except that a is only evaluated once.
+
+                var leftOperand = node.LeftOperand;
+                if (!TryGetNullableMethod(leftOperand.Syntax,


if (!TryGetNullableMethod(leftOperand.Syntax, [](start = 16, length = 45)

Do we have tests for those missing methods? #Closed

See CodeGenNullCoalescingAssignmentTests.MissingMembers

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

jcouv · 2019-06-12T20:04:24Z

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

+
+                // If lhsRead is a call, such as to a property accessor, save the result off to a temp. This doesn't affect
+                // the standard ??= case because it only uses lhsRead once.
+                if (lhsRead.Kind == BoundKind.Call)


if (lhsRead.Kind == BoundKind.Call) [](start = 16, length = 35)

I didn't understand this. I would have expected TransformCompoundAssignmentLHS to already have done what's needed, and we can just use transformedLHS that it returned.
FWIW, other scenarios that use TransformCompoundAssignmentLHS do not special-case calls. #Closed

Correct, because other compound assignments do not have to read the value twice like we have to. It leaves calls to property accessors as is, so if we don't store the value to a temp we'll end up calling the property accessor twice.

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

jcouv · 2019-06-12T20:10:09Z

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

+                // We lower the expression to this form:
+                //
+                // var tmp = lhsRead.GetValueOrDefault();
+                // if (!lhsRead.HasValue) { tmp = loweredRight; transformedAssignment = tmp; }


transformedAssignment [](start = 64, length = 21)

nit: should be transformedLhs for consistency with comments below #Closed

jcouv · 2019-06-12T20:11:31Z

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

+                // !lhsRead.HasValue
+                var lhsReadHasValue = BoundCall.Synthesized(leftOperand.Syntax, lhsRead, hasValue);
+                var lhsReadHasValueNegation =
+                    MakeUnaryOperator(UnaryOperatorKind.BoolLogicalNegation, leftOperand.Syntax, method: null, lhsReadHasValue, lhsReadHasValue.Type);


MakeUnaryOperator [](start = 20, length = 17)

Can this be _factory.Not(...)? #Closed

I didn't know about these, thanks.

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

jcouv · 2019-06-12T20:11:59Z

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

+
+                // { tmp = b; transformedLhs = tmp; }
+                var consequenceBuilder = ArrayBuilder<BoundStatement>.GetInstance(2);
+                consequenceBuilder.Add(new BoundExpressionStatement(node.Syntax, tmpAssignment) { WasCompilerGenerated = true });


BoundExpressionStatement [](start = 43, length = 24)

Consider using _factory.ExpressionStatement(...). Also below #Closed

jcouv · 2019-06-12T20:13:03Z

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

+                var consequenceBuilder = ArrayBuilder<BoundStatement>.GetInstance(2);
+                consequenceBuilder.Add(new BoundExpressionStatement(node.Syntax, tmpAssignment) { WasCompilerGenerated = true });
+                consequenceBuilder.Add(new BoundExpressionStatement(node.Syntax, transformedLhsAssignment) { WasCompilerGenerated = true });
+                var consequence = new BoundBlock(node.Syntax, locals: ImmutableArray<LocalSymbol>.Empty, consequenceBuilder.ToImmutableAndFree())


BoundBlock [](start = 38, length = 10)

Consider using _factor.Block(statement1, statement2) #Closed

jcouv · 2019-06-12T20:19:09Z

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

+                statementsBuilder.Add(ifHasValueStatement);
+
+                _needsSpilling = true;
+                return _factory.SpillSequence(temps.ToImmutableAndFree(), statementsBuilder.ToImmutableAndFree(), tmp);


SpillSequence [](start = 32, length = 13)

Technically, we could do without the spilling, just using a Sequence. I don't know if it's worth it though.

// no statements, only expressions and side-effects in the following: // var tmp = lhsRead.GetValueOrDefault(); // !lhsRead.HasValue ? { /*sequence*/ tmp = loweredRight; transformedAssignment = tmp; tmp } : tmp ``` #Closed

I think this version is likely more efficient IL wise, since it doesn't have the multiple tmps at the end of a ternary. Also, it's the version that LDM approved, so I'd prefer to stick with the existing one.

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

I don't think LDM approved any specific IL-level lowered form. I recall we only discussed a C#-level lowered form and also guiding principles.
Don't forget that the lowered form here (described at the top of this local function) is not the final lowered form anyways, as the spilling will manipulate it further. Also, spilling makes this harder to reason about (exactly what is spilled? what is the final form?) and required some changes to TransformCompoundAssignmentLHS which I'm a bit concerned about.

I'm curious what Neal, Aleksey or Chuck think about this.

In reply to: 293103887 [](ancestors = 293103887,293099162)

jcouv · 2019-06-12T20:21:39Z

...Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs

        /// The returned node needs to be lowered but its children are already lowered.
        /// </returns>
-        private BoundExpression TransformCompoundAssignmentLHS(BoundExpression originalLHS, ArrayBuilder<BoundExpression> stores, ArrayBuilder<LocalSymbol> temps, bool isDynamicAssignment)
+        private BoundExpression TransformCompoundAssignmentLHS(BoundExpression originalLHS, ArrayBuilder<BoundExpression> stores, ArrayBuilder<LocalSymbol> temps, bool isDynamicAssignment, bool needsSpilling = false)


bool needsSpilling = false) [](start = 189, length = 27)

As commented in lowering of null-coalescing operator, I'm not sure we need spilling. Changes in this file feel unnecessary. #Closed

agocke · 2019-06-11T23:59:15Z

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

+            if (leftType.IsNullableType())
            {
-                Error(diagnostics, ErrorCode.WRN_DefaultLiteralConvertedToNullIsNotIntended, node.Right, leftType.GetNullableUnderlyingType());
+                // If B is implicitly convertible to A0, then the result type of this expression is A0, except if B is dynamic.


It looks like this comment conflicts with the comment above. Also, you've lost the definition of A0 #Resolved

agocke · 2019-06-12T20:35:55Z

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

+                    diagnostics.Add(node, useSiteDiagnostics);
+                    var convertedRightOperand = CreateConversion(rightOperand, underlyingRightConversion, underlyingLeftType, diagnostics);
+                    // Create a nullable conversion for use by the local rewriter
+                    var nullableConversion = Conversions.ClassifyImplicitConversionFromType(underlyingLeftType, leftType, ref useSiteDiagnostics);


Not sure I understand what's going on here. Are we throwing away the conversion? Don't we need to attach it to something for the LocalRewriter? #Resolved

See the comment to Julien. I was using this at one point and just forgot to remove it.

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

agocke · 2019-06-12T20:49:07Z

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

+                // the standard ??= case because it only uses lhsRead once.
+                if (lhsRead.Kind == BoundKind.Call)
+                {
+                    var lhsTemp = _factory.StoreToTemp(lhsRead, out var store, kind: SynthesizedLocalKind.Spill);


Why do we need SynthesizedLocalKind.Spill? #Resolved

When we get to the SpillSequenceSpiller, it asserts that all synthesized locals are long lived. The default (LoweringTemp) is not.

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

I see no good reason why we need this temporary to be long lived, since the other ones used in compound assignments are not. #Resolved

The other ones are not used in a spill sequence, this is.

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

agocke · 2019-06-12T20:55:11Z

...Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs

            // we have a variable on the left. Transform it into:
            // ref temp = ref variable
            // temp = temp + value
+            var localKind = needsSpilling ? SynthesizedLocalKind.Spill : SynthesizedLocalKind.LoweringTemp;


Seems worrying if we create a ref temp that needs spilling since that's not possible. #Resolved

Turns out this is an existing problem with compound assignment in general. Filed #36443

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

jcouv · 2019-06-12T21:14:05Z

    /// <param name="isDynamicAssignment">True if the compound assignment is a dynamic operation.</param>

nit: Let's xml doc the new parameter: needsSpilling should be true when the variable produced by TransformCompoundAssignmentLHS for the LHS should have "spill" kind. #Closed

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs:461 in 6977857. [](commit_id = 6977857, deletion_comment = False)

jcouv · 2019-06-12T21:15:36Z

...Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_CompoundAssignmentOperator.cs

                            var loweredIndices = VisitList(arrayAccess.Indices);

-                            return SpillArrayElementAccess(loweredArray, loweredIndices, stores, temps);
+                            return SpillArrayElementAccess(loweredArray, loweredIndices, stores, temps, localKind);


localKind); [](start = 104, length = 11)

I'm assuming that all the cases in this switch are covered by a nullable value type scenario. If not, let's add test coverage. #Closed

Added array accesses. We had some for indexers, but not for regular arrays. Everything else should be covered already.

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

jcouv · 2019-06-12T21:24:21Z

src/Test/Utilities/Portable/Compilation/ControlFlowGraphVerifier.cs

                                ((isFirstOperandOfDynamicOrUserDefinedLogicalOperator(reference) ||
                                     isIncrementedNullableForToLoopControlVariable(reference) ||
                                     isConditionalAccessReceiver(reference) ||
+                                     isCoalesceAssignment(reference) ||


isCoalesceAssignment [](start = 37, length = 20)

Could we tighten this exception to only be for null-coalescing assignments where the LHS is a property? #Closed

I've updated the syntax used in the CFG to be a little more precise and removed this entirely.

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

To be clear, I actually didn't need it last iteration, I had left it in but had apparently already removed the need.

In reply to: 294962493 [](ancestors = 294962493,293124296)

jcouv · 2019-06-12T21:29:01Z

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

-
 .locals {R1}
 {
-    CaptureIds: [0]


[0] [](start = 16, length = 3)

Just curious: what previously caused only one capture to be reported here? (we already had IFlowCaptureOperation: 1 below) #Closed

The capture was more locally scoped to just Region 2.

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

jcouv · 2019-06-12T21:42:54Z

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

+                // We lower the expression to this form:
+                //
+                // var tmp = lhsRead.GetValueOrDefault();
+                // if (!lhsRead.HasValue) { tmp = loweredRight; transformedAssignment = tmp; }


!lhsRead.HasValue [](start = 23, length = 17)

~~Should this rather be a null test on tmp?~~ Nevermind. Got confused. #Closed

jcouv · 2019-06-12T21:44:51Z

src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs

+                if (!isStatement)
+                {
+                    intermediateFrame = PushStackFrame();
+                    SpillEvalStack();


SpillEvalStack [](start = 20, length = 14)

not related to this PR: I feel like I forgot what most of those utility methods do since last time I reviewed some CFG work. It may be worth investing in some documentation for CFG so that we can reload some context in future reviews. #Closed

jcouv · 2019-06-12T22:04:57Z

src/Compilers/Core/Portable/Operations/ControlFlowGraphBuilder.cs

+
+            return isStatement ? null : GetCaptureReference(resultCaptureId, operation);
+
+            void nullableValueTypeReturn()


nullableValueTypeReturn [](start = 17, length = 23)

// var tmp = lhsRead.GetValueOrDefault(); // if (!lhsRead.HasValue) { tmp = loweredRight; transformedAssignment = tmp; } // tmp // OR for statement // if (!lhsRead.HasValue) { transformedAssignment = loweredRight; }

Suggestion: If the comment referenced block names (afterCoalesce and such), that'd be even better. #Closed

jcouv · 2019-06-12T22:45:47Z

    Console.WriteLine(i1 ??= GetInt());

Consider adding a test for (i1 ??= GetInt()).HasValue which produces an error because the result type no longer is int?. #Closed

Refers to: src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenNullCoalescingAssignmentTests.cs:29 in 6977857. [](commit_id = 6977857, deletion_comment = False)

jcouv · 2019-06-12T22:50:36Z

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

    {
        var c = new C();
        Console.WriteLine(c.f1 ??= GetInt());
+        Console.WriteLine(c.f2 ??= GetObject());


Console.WriteLine(c.f2 ??= GetObject()); [](start = 7, length = 41)

Consider extracting both WriteLine statements to separate methods for clarity in VerifyIL.
Also applies in other tests #Closed

jcouv · 2019-06-12T22:53:21Z

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

+    public static void TestObject()
+    {
+        var c = new C();
+        Console.WriteLine(c.P2 ??= GetInt(1));


1 [](start = 42, length = 1)

Consider incrementing here (use 3 and 4 instead of 1 and 2 again) #Closed

jcouv

Done with review pass (iteration 2)
I'm still thinking about lowering with statement+spilling vs. lowering without statement.

333fred · 2019-06-18T18:23:11Z

    Console.WriteLine(i1 ??= GetInt());

I don't think this is necessary. I already have several tests that show that type of this is int, not int?, and adding this case doesn't test anything new.

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

Refers to: src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenNullCoalescingAssignmentTests.cs:29 in 6977857. [](commit_id = 6977857, deletion_comment = False)

…ound a bug in TransformCompoundAssignmentLHS. Addressed PR feedback.

333fred · 2019-06-18T21:29:57Z

@agocke @jcouv I've addressed your feedback. I've moved to Julien's suggested lowering form for now, and filed #36548 to follow up on it later.

jcouv

LGTM Thanks (iteration 3)

…-types * dotnet/master: (63 commits) Fix stack overflow in requesting syntax directives (dotnet#36347) crash on ClassifyUpdate for EventFields (dotnet#35962) Disable move type when the options service isn't present (dotnet#36334) Fix crash where type inference doing method inference needs to drop nullability Fix parsing bug in invalid using statements (dotnet#36428) Do not suggest or diagnose use compound assignment when right hand of binary operator is a throw expression Add option to emit nullable metadata for public members only (dotnet#36398) Added null checks on F# external access services (dotnet#36469) Deal with discovering extra .editorconfig files Re-enable MSBuildWorkspaceTests.TestEditorConfigDiscovery Add support to VisualStudioMSBuildInstalled to support minimum versions Fix configuration of accessibilities in editorconfig Shorten a resource ID Revert "Extract the RDT implementation for Misc files and VS open file tracker" Add nullability support to use local function Add EditorFeatures.WPF dependency to F# ExternalAccess Ensure NullableWalker.AsMemberOfType locates the right new container for the member. (dotnet#36406) Replace `dynamic` with `object` when substituting constraints. (dotnet#36379) Add some string descriptions Adjust type of out var based on parameter state (dotnet#36284) ...

agocke

LGTM

333fred added 2 commits June 11, 2019 11:00

Update ??= to convert the result type of the expression to the underl…

da58238

…ying value type when targetting a nullable value type.

Update control flow graph builder with changes to ??=.

6977857

333fred requested review from a team and cston June 11, 2019 18:14

333fred mentioned this pull request Jun 11, 2019

Champion "Null-coalescing assignments" (16.3, Core 3) dotnet/csharplang#34

Open

5 tasks

jcouv added the Area-Compilers label Jun 11, 2019

jcouv added this to the 16.2 milestone Jun 11, 2019

jcouv reviewed Jun 12, 2019

View reviewed changes

agocke reviewed Jun 12, 2019

View reviewed changes

jcouv reviewed Jun 12, 2019

View reviewed changes

jcouv self-assigned this Jun 12, 2019

jcouv modified the milestones: 16.2, 16.2.P4 Jun 12, 2019

Move to a different lowering from for nullable value types to work ar…

3f04a07

…ound a bug in TransformCompoundAssignmentLHS. Addressed PR feedback.

jcouv approved these changes Jun 18, 2019

View reviewed changes

agocke approved these changes Jun 18, 2019

View reviewed changes

333fred merged commit 05b1ec7 into dotnet:master Jun 19, 2019

333fred deleted the null-coalesce-value-types branch June 19, 2019 07:56


		return isStatement ? null : GetCaptureReference(resultCaptureId, operation);

		void nullableValueTypeReturn()

Conversation

333fred commented Jun 11, 2019

Uh oh!

333fred commented Jun 12, 2019 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcouv commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv commented Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcouv Jun 12, 2019 • edited by 333fred Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred commented Jun 12, 2019

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jcouv Jun 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

333fred commented Jun 12, 2019 •

edited by jcouv

Loading

jcouv commented Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv commented Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited by 333fred

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading

agocke Jun 11, 2019 •

edited by 333fred

Loading

agocke Jun 12, 2019 •

edited by 333fred

Loading

agocke Jun 12, 2019 •

edited by 333fred

Loading

agocke Jun 12, 2019 •

edited by 333fred

Loading

agocke Jun 12, 2019 •

edited by 333fred

Loading

jcouv commented Jun 12, 2019 •

edited by 333fred

Loading

jcouv Jun 12, 2019 •

edited

Loading

jcouv Jun 12, 2019 •

edited

Loading