Use synthesized attribute infrastructure for System.NullableAttribute by cston · Pull Request #22820 · dotnet/roslyn

cston · 2017-10-24T10:22:11Z

No description provided.

cston · 2017-10-24T15:57:30Z

@dotnet/roslyn-compiler, @OmarTawfik, @AlekseyTs please review. #Resolved

OmarTawfik · 2017-10-24T16:36:46Z

src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs

-            }
+            return new SynthesizedAttributeData(
+                _lazyEmbeddedAttribute.Constructors[0],
+                ImmutableArray<TypedConstant>.Empty,


Thinking out loud: Is there another way of selecting the correct constructor rather than index? what happens if later on another constructor was added?
Can we filter based on signature?

Index seems like a bad idea. In general the order of metadata is "unordered" hence depending on a specific index seems wrong. Maybe something i'm missing here that makes this safer.

_lazy*Attribute are SynthesizedEmbeddedAttribute instances. The constructors are an immutable collection created with a fixed order.

jcouv · 2017-10-24T16:48:25Z

src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs

-                    ImmutableArray<KeyValuePair<string, TypedConstant>>.Empty);
-            }
+            return new SynthesizedAttributeData(
+                _lazyEmbeddedAttribute.Constructors[0],


_lazyEmbeddedAttribute [](start = 16, length = 22)

Is it still possible for _lazyEmbeddedAttribute to be null? #Closed

It would be a bug to call SynthesizeEmbeddedAttribute without setting _lazyEmbeddedAttribute first. Previously, we were relying on base.SynthesizeEmbeddedAttribute() which threw ExceptionUtilities.Unreachable. #Resolved

From discussion with Chuck, we need to decide if NullableAttribute should only be embedded, or whether we could add it to frameworks at some point.
Btw, my assumption was it would become part of frameworks.

jcouv · 2017-10-24T16:57:28Z

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


        private bool _needsGeneratedIsReadOnlyAttribute_IsFrozen;
        private bool _needsGeneratedIsByRefLikeAttribute_IsFrozen;
+        private bool _needsGeneratedNullableAttribute_IsFrozen;


It seems that we use the IsFrozen flags to enforce a phase ordering constraint. Do we really need one flag per kind of attribute, or could we do with a single one? #Closed

It looks like we only need a single flag. Updated. #Resolved

jcouv · 2017-10-24T17:21:13Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

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


Consider testing:
~~class C : List<object?>~~ (already there)
~~class C : I<object?>~~ (already there)
class C<T> where T : object?
class C<T?> if that's allowed
What happens if NullableAttribute exists in source, or in a reference?
Consider verifying the emitted attribute (with serialized booleans) in some cases, as we discussed. #Resolved

jcouv · 2017-10-24T17:25:31Z

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

@@ -499,11 +499,15 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r

            if (type.ContainsNullableReferenceTypes())


Not related to this PR: it seems ContainsNullableReferenceTypes is implemented by manually visiting type symbols. Is it possible to use VisitType like ContainsDynamic and ContainsTupleNames?
Maybe this needs to wait until we try to merge type annotations into type symbols (assuming that nullability will still be part of binding, not just handled by flow analysis)? #Closed

Added a // PROTOTYPE comment to TypeSymbol.ContainsNullableReferenceTypes. #Resolved

jcouv · 2017-10-24T17:39:13Z

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

            }
        }

+        internal void EnsureNullableAttributeExists(DiagnosticBag diagnostics, Location location, bool modifyCompilationForNullable)


I couldn't find where we pass modifyCompilationForNullable as false. Am I missing something? #Closed

The argument was not used previously but should have been used for bound lambdas. Updated. #Closed

jcouv · 2017-10-24T17:41:47Z

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

+            {
+                // PROTOTYPE(NullableReferenceTypes): Create attribute
+                // with PEModuleBuilder.SynthesizeNullableAttribute().
+                // See AttributeTests_Nullable.EmitAttribute_Interface.


Could you add a note to test a mix of tuple names and nullability? The code skeleton looks like it will not handle that. #Closed

Thanks. Updated extension method and unit test. #Resolved

jcouv · 2017-10-24T17:56:00Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+            {
+                var type = module.ContainingAssembly.GetTypeByMetadataName("C");
+                var property = (PropertySymbol)type.GetMembers("this[]").Single();
+                AssertNullableAttribute(property.Parameters[1].GetAttributes());


Consider asserting no nullable attribute on Parameters[0] #Closed

Added. #Resolved

jcouv · 2017-10-24T18:19:06Z

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

            get
            {
-                // PROTOTYPE(NullableReferenceTypes): C#8 projects require System.Attribute.
                return _assemblySymbol.DeclaringCompilation.IsFeatureEnabled(MessageID.IDS_FeatureStaticNullChecking);


Is the plan to remove UtilizesNullableReferenceTypes altogether? If not, can you clarify what it should mean? #Closed

There is a // PROTOTYPE comment on the base method in ModuleSymbol to remove this property. #Resolved

jcouv · 2017-10-24T18:20:29Z

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

            ParameterHelpers.EnsureIsReadOnlyAttributeExists(Parameters, diagnostics, modifyCompilationForRefReadOnly: true);
+
+            this.EnsureNullableAttributeExistsIfNecessary(ReturnType, diagnostics);
+            ParameterHelpers.EnsureNullableAttributeExistsIfNecessary(Parameters, diagnostics);


This PR doesn't change local function or lambda symbols. Should it? #Closed

Good catch. Added. #Closed

jcouv · 2017-10-24T18:22:25Z

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

+        {
+            if (type.ContainsNullableReferenceTypes())
+            {
+                symbol.DeclaringCompilation.EnsureNullableAttributeExists(diagnostics, symbol.Locations[0], modifyCompilationForNullable: true);


Are we sure symbol.Location.Length is always > 0? #Closed

No, Locations[0] was not correct. Fixed. #Resolved

jcouv · 2017-10-24T18:24:36Z

src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs

            }
        }

+        internal bool NeedsGeneratedNullableAttribute


Is this property used? #Closed

There is one caller, PEAssemblyBuilder.CreateEmbeddedAttributesIfNeeded, similar to NeedsGeneratedIsReadOnlyAttribute and NeedsGeneratedIsByRefLikeAttribute. I've changed the properties to protected so it's more obvious where the properties are used. #Closed

jcouv · 2017-10-25T19:24:28Z

src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs

+            // Also freeze generated attribute flags.
+            // Symbols bound after getting the declaration
+            // diagnostics shouldn't need to modify the flags.
+            _needsGeneratedAttributes_IsFrozen = true;


Not blocking this PR: this raises the follow-up question: are declaration diagnostics and generated attributes getting frozen at the same phase?

What did you think about this? Should we only have one "frozen" flag here too?

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

Logged #22887.

jcouv · 2017-10-25T20:51:47Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+                parseOptions: TestOptions.Regular8);
+            comp.VerifyEmitDiagnostics(
+                // error CS0518: Predefined type 'System.Attribute' is not defined or imported
+                Diagnostic(ErrorCode.ERR_PredefinedTypeNotFound).WithArguments("System.Attribute").WithLocation(1, 1),


This is probably not a new issue, or related to nullable feature, but I wonder if we could tie this diagnostic to the first syntax that triggered it.
Should I file a follow-up issue? #Closed

The two errors in the test are use-site errors when failing to create two distinct synthesized attribute types: NullableAttribute and EmbeddedAttribute. In this case, the use-site errors happen to be the same error. We will only generate one error per synthesized attribute type declaration for a single compilation though. #Resolved

jcouv · 2017-10-25T21:19:06Z

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

 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using System.Linq;


Do we need this using? #Closed

Used for Parameters.Any(). #Closed

My bad. I thought ImmutableArrayExtensions was in one of our namespaces. Thanks #Closed

jcouv · 2017-10-25T21:21:23Z

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

+            {
+                if (parameter.Type.ContainsNullableReferenceTypes())
+                {
+                    parameter.DeclaringCompilation.EnsureNullableAttributeExists(diagnostics, parameter.GetNonNullSyntaxNode().Location, modifyCompilation);


Method above handles null DeclarationCompilation. That's not needed here? #Closed

jcouv · 2017-10-25T21:24:53Z

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

+                Location GetReturnTypeLocation()
+                {
+                    var syntax = (DelegateDeclarationSyntax)SyntaxRef.GetSyntax();
+                    return syntax.ReturnType.GetLocation();


Nit: empty line below #Closed

jcouv · 2017-10-25T21:25:59Z

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


            internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, DiagnosticBag diagnostics)
            {
+                Location GetReturnTypeLocation()


Nit: I think we've used camelCase for local functions in compiler code so far. #Closed

jcouv · 2017-10-25T21:29:13Z

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


        internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions, DiagnosticBag diagnostics)
        {
+            Location GetReturnTypeLocation() => GetSyntax().ReturnType.Location;


Nit: same comment on camelCasing #Closed

AlekseyTs · 2017-10-27T22:45:11Z

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

+                diagnosticsOpt,
+                locationOpt,
+                WellKnownType.System_Runtime_CompilerServices_NullableAttribute,
+                WellKnownMember.System_Runtime_CompilerServices_NullableAttribute__ctor);


System_Runtime_CompilerServices_NullableAttribute__ctor [](start = 32, length = 55)

It looks like this attribute has two constructors. Should we check both of them? #Closed

Yes. See AttributeTests_Nullable.ExplicitAttribute_MissingConstructor.

AlekseyTs · 2017-10-27T22:48:49Z

src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs

+        {
+            var constructorIndex = (member == WellKnownMember.System_Runtime_CompilerServices_NullableAttribute__ctorTransformFlags) ? 1 : 0;
+            return new SynthesizedAttributeData(
+                _lazyNullableAttribute.Constructors[constructorIndex],


_lazyNullableAttribute [](start = 16, length = 22)

It looks like this code is going to cause a NullReferenceException in case when the attribute doesn't have to be embedded. In this case we should fall back to the regular way of synthesizing attributes by using already available type. #Closed

Thanks. I will handle existing attribute types in a separate PR. See ExplicitAttributeFromSource test.

I will handle existing attribute types in a separate PR.

Why? It feels like we should simply follow the pattern of the other similar methods, i.e. call the base when _lazyNullableAttribute is null. Is there something special about Nullable attribute that I am missing. Is there a reason to deviate? #Closed

More tests will be required when supporting existing NullableAttribute definitions, testing for missing constructors, etc.

AlekseyTs · 2017-10-27T22:53:04Z

src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs

+            NamedTypeSymbol containingType,
+            DiagnosticBag diagnostics)
+        {
+            var boolType = compilation.GetSpecialType(SpecialType.System_Boolean);


GetSpecialType [](start = 39, length = 14)

~~Use-site diagnostics?~~ #Closed

AlekseyTs · 2017-10-27T23:02:40Z

src/Compilers/CSharp/Portable/Emitter/Model/PEModuleBuilder.cs

            return Compilation.TrySynthesizeAttribute(WellKnownMember.System_Runtime_CompilerServices_IsByRefLikeAttribute__ctor);
        }

+        protected virtual MethodSymbol GetSynthesizedAttributeConstructor(WellKnownMember constructor)


GetSynthesizedAttributeConstructor [](start = 39, length = 34)

It doesn't look like this method is used. #Closed

Thanks. Removed.

AlekseyTs · 2017-10-27T23:12:24Z

src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedImplementationMethod.cs

            if (returnType.ContainsNullableReferenceTypes())
            {
-                AddSynthesizedAttribute(ref attributes, compilation.SynthesizeNullableAttribute(returnType));
+                AddSynthesizedAttribute(ref attributes, moduleBuilder.SynthesizeNullableAttribute(this, returnType));


AddSynthesizedAttribute(ref attributes, moduleBuilder.SynthesizeNullableAttribute(this, returnType)); [](start = 16, length = 101)

Where does this attribute go? Shouldn't it be applied to a return type instead? #Closed

Please add a unit-test that hits this code path.

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

Added EmitAttribute_ExplicitImplementationForwardingMethod.

Added EmitAttribute_ExplicitImplementationForwardingMethod

It doesn't look like that test verifies the result of this method. I believe this method adds attributes on the method itself, rather than on the return type of the method, which is incorrect. I think we should simply remove this override of AddSynthesizedAttributes and open an issue to do the same in the "parent" branch. #Closed

Logged #22952.

AlekseyTs · 2017-10-27T23:23:01Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+}";
+            var comp = CreateStandardCompilation(source, parseOptions: TestOptions.Regular8);
+            comp.VerifyEmitDiagnostics(
+                // error CS0518: Predefined type 'System.Boolean' is not defined or imported


// error CS0518: Predefined type 'System.Boolean' is not defined or imported [](start = 16, length = 76)

Doesn't CreateStandardCompilation add mscorlib reference? #Closed

Updated test.

AlekseyTs · 2017-10-27T23:23:14Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+}";
+            var comp = CreateStandardCompilation(source, references: new[] { ref0 }, parseOptions: TestOptions.Regular8);
+            comp.VerifyEmitDiagnostics(
+                // error CS0518: Predefined type 'System.Boolean' is not defined or imported


// error CS0518: Predefined type 'System.Boolean' is not defined or imported [](start = 16, length = 76)

Doesn't CreateStandardCompilation add mscorlib reference? #Closed

Updated test.

AlekseyTs · 2017-10-27T23:35:01Z

Done with review pass (iteration 10). #Closed

cston · 2017-10-31T17:12:03Z

@dotnet/roslyn-compiler, @AlekseyTs, please provide a second review, thanks.

AlekseyTs · 2017-10-31T17:36:31Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+                    // Generated iterator methods have no [Nullable] attributes.
+                    // Since the methods are only called through IEnumerable<T>
+                    // or IEnumerator<T>, that is not an issue.
+                    AssertNoNullableAttribute(property.GetAttributes());


AssertNoNullableAttribute(property.GetAttributes()); [](start = 20, length = 52)

Please test attributes on the accessor's return type as well. #Closed

AlekseyTs · 2017-10-31T17:56:21Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+                symbolValidator: module =>
+                {
+                    var method = module.ContainingAssembly.GetTypeByMetadataName("B").GetMethod("I.F");
+                    AssertNullableAttribute(method.GetReturnTypeAttributes());


AssertNullableAttribute(method.GetReturnTypeAttributes()); [](start = 20, length = 58)

Please assert that we don't have the attribute on the method itself, same for other test scenarios. #Closed

AlekseyTs · 2017-10-31T18:05:16Z

Done with review pass (iteration 13). #Closed

AlekseyTs · 2017-10-31T18:17:28Z

It looks like Microsoft.CodeAnalysis.CSharp.UnitTests.CodeGen.CodeGen_DynamicTests.Iterator needs a base-line adjustment #Closed

cston · 2017-11-02T03:38:55Z

@AlekseyTs, all feedback should be addressed.

AlekseyTs · 2017-11-02T17:25:23Z

src/ExpressionEvaluator/CSharp/Source/ExpressionCompiler/Symbols/EEMethodSymbol.cs

-                    AddSynthesizedAttribute(ref attributes, compilation.SynthesizeNullableAttribute(this.ReturnType));
-                }
-                diagnostics.Free();
+                AddSynthesizedAttribute(ref attributes, moduleBuilder.SynthesizeNullableAttribute(this, this.ReturnType));


AddSynthesizedAttribute(ref attributes, moduleBuilder.SynthesizeNullableAttribute(this, this.ReturnType)); [](start = 16, length = 106)

It looks like this line is going to add a duplicate attribute, the base class adds one too. Please add a unit-test to ensure this is not happening. #Closed

Added ExpressionCompilerTests.EmitNullableAttribute_ExpressionType test. That test is failing for a distinct reason however.

AlekseyTs · 2017-11-02T17:37:26Z

src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests_Nullable.cs

+                    // or IEnumerator<T>, that is not an issue.
+                    AssertNoNullableAttribute(property.GetAttributes());
+                    var method = property.GetMethod;
+                    AssertNoNullableAttribute(method.GetReturnTypeAttributes());


AssertNoNullableAttribute(method.GetReturnTypeAttributes()); [](start = 20, length = 60)

This feels unexpected. Does it mean that we are dropping nullability of return type on the symbol itself? What happens when nullable modifier is nested, i.e. the type is an array of nullable objects? #Closed

It looks like we are dropping nullability on the top-level type. Added a test for IEnumerable<object?[]> and a // PROTOTYPE comment for now.

AlekseyTs · 2017-11-02T17:42:54Z

Done with review pass (iteration 14). #Closed

AlekseyTs

LGTM (iteration 15).

…dotnet#22820) Use synthesized attribute infrastructure for System.NullableAttribute Misc. Misc. Misc. Misc. Misc. Misc. PR feedback Override in PEMethodSymbol

dnfclas added the cla-already-signed label Oct 24, 2017

Use synthesized attribute infrastructure for System.NullableAttribute

49263e2

cston force-pushed the embedded branch from cc5012f to 49263e2 Compare October 24, 2017 15:55

OmarTawfik reviewed Oct 24, 2017

View reviewed changes

jcouv reviewed Oct 24, 2017

View reviewed changes

cston mentioned this pull request Oct 24, 2017

[Work items] Nullable reference types #22152

Closed

cston force-pushed the embedded branch from 1957996 to ce5e07c Compare October 25, 2017 15:47

PR feedback

04841d8

cston force-pushed the embedded branch from ce5e07c to 04841d8 Compare October 25, 2017 18:45

jcouv reviewed Oct 25, 2017

View reviewed changes

cston added 3 commits October 25, 2017 13:10

Misc.

96609d0

Fix test

00d052d

Rename method

2c1670a

jcouv reviewed Oct 25, 2017

View reviewed changes

AlekseyTs reviewed Oct 27, 2017

View reviewed changes

cston added 3 commits October 27, 2017 21:46

PR feedback

7a908fe

PR feedback

93a14eb

Fix compile error

9c41e2b

AlekseyTs reviewed Oct 31, 2017

View reviewed changes

PR feedback

76f538b

cston mentioned this pull request Nov 1, 2017

Remove SynthesizedImplementationMethod.AddSynthesizedAttributes #22952

Closed

AlekseyTs reviewed Nov 2, 2017

View reviewed changes

More PR feedback

beb6a4a

AlekseyTs approved these changes Nov 2, 2017

View reviewed changes

cston merged commit 7a0467e into dotnet:features/NullableReferenceTypes Nov 2, 2017

cston deleted the embedded branch November 2, 2017 22:44

jcouv added the Feature - Nullable Reference Types Nullable Reference Types label Nov 8, 2017

cston mentioned this pull request Jul 27, 2018

Report error for T? when T is unconstrained #28804

Merged

		@@ -499,11 +499,15 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r

		if (type.ContainsNullableReferenceTypes())

Conversation

cston commented Oct 24, 2017

Uh oh!

cston commented Oct 24, 2017 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

OmarTawfik Oct 24, 2017

Choose a reason for hiding this comment

Uh oh!

jaredpar Oct 24, 2017

Choose a reason for hiding this comment

Uh oh!

cston Oct 24, 2017 • 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 Oct 24, 2017 • 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!

cston Oct 24, 2017 • edited by jcouv 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 Oct 24, 2017 • 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 Oct 24, 2017 • 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!

cston Oct 24, 2017 • edited by jcouv 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 Oct 24, 2017 • 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 Oct 24, 2017 • 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!

cston Oct 24, 2017 • edited by jcouv 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 Oct 24, 2017 • 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!

cston Oct 25, 2017 • edited by jcouv 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 Oct 24, 2017 • 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!

cston Oct 25, 2017 • edited by jcouv 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 Oct 24, 2017 • 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!

cston Oct 25, 2017 • edited by jcouv 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 Oct 24, 2017 • 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!

cston Oct 24, 2017 • edited by jcouv Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cston commented Oct 24, 2017 •

edited by jcouv

Loading

cston Oct 24, 2017 •

edited

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 24, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 24, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 24, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 24, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 24, 2017 •

edited

Loading

cston Oct 24, 2017 •

edited by jcouv

Loading

jcouv Oct 25, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 25, 2017 •

edited

Loading

cston Oct 25, 2017 •

edited by jcouv

Loading

jcouv Oct 25, 2017 •

edited

Loading

jcouv Oct 25, 2017 •

edited

Loading

jcouv Oct 25, 2017 •

edited

Loading

jcouv Oct 25, 2017 •

edited

Loading