Help compiler enforce nullability annotations #32090

jcouv · 2020-02-11T03:17:41Z

TL;DR: the goal for this PR is to confirm that some upcoming compiler changes for VS 16.6 are okay. ~~This PR is intended for discussion but not to merge as-is :-)~~

Tagging @stephentoub @safern for discussion.

Background:

I have been working on compiler changes which adds some warnings as a result of nullability attributes.

For instance:

[MaybeNull] string parameter would start with a maybe-null state.
[MaybeNull] override T M() cannot override virtual T M()
we check the state of [MaybeNullWhen(true)] string parameter
we check that methods with [DoesNotReturn] indeed don't return

Jared recommended that I test the change on the runtime repo, and it indeed produced more warnings. Here's a PR that addresses some of the issues flagged and tags some others.

Issues identified so far:

some GetHashCode methods are marked as [DisallowNull] and that's causing problems
FailFast is marked with [DoesNotReturn], but it looks like it could return in some cases
I've tagged a few places where methods with conditional [Maybe/NotNullWhen(...)] attributes seem incorrectly implemented

jcouv · 2020-02-11T04:26:35Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -221,7 +221,7 @@ public ImmutableHashSet<T> Remove(T item)
        [Pure]
        public bool TryGetValue(T equalValue, out T actualValue)
        {
-            int hashCode = _equalityComparer.GetHashCode(equalValue);
+            int hashCode = _equalityComparer.GetHashCode(equalValue!);


📝 should we remove the [DisallowNull] on that GetHashCode, or add [DisallowNull] here?
(same question below) #Resolved

In general IEqualityComparer<T>.GetHashCode (e.g. StringComparer.GetHashCode throws ArgumentNullException for null) even if some implementations do, so we can't remove [DisallowNull]. We could consider adding [DisallowNull] to this T equalValue, but then we'd be saying that null wasn't allowed even if the developer provided a comparer implementation that did actually allow null. This is an example where we lack sufficient expressivity. I guess for now it'd be best to add [DisallowNull], and then see what kind of issues that causes; we can remove [DisallowNull] later on, but it'll be harder to add it later on. #Resolved

jcouv · 2020-02-11T04:27:18Z

...del.Composition/src/System/ComponentModel/Composition/Primitives/ComposablePartDefinition.cs

@@ -140,10 +140,12 @@ internal virtual bool TryGetExports(ImportDefinition definition, [NotNullWhen(tr
            if (multipleExports != null)
            {
                multipleMatches = multipleExports;
+                // TODO2 singleMatch dould be null when returning true
+                singleMatch = null!;


📝 the [NotNullWhen(...)] annotation on method seems incorrect #Resolved

Yes, please remove it. Thanks. #Resolved

jcouv · 2020-02-11T04:27:38Z

...mponentModel.Composition/src/System/ComponentModel/Composition/ReflectionModel/ImportType.cs

@@ -172,14 +172,15 @@ private static bool IsLazyGenericType(Type genericType)

        private static bool TryGetCastFunction(Type genericType, bool isOpenGeneric, Type[] arguments, [NotNullWhen(true)] out Func<Export, object>? castFunction)
        {
-            castFunction = null;
+            castFunction = null!; // TODO2


📝 the [NotNullWhen(...)] annotation on method seems incorrect #Resolved

Yes, please remove it. The dangers of the compiler not having checked those annotations ;) #Resolved

jcouv · 2020-02-11T04:27:50Z

.../src/System/ComponentModel/Composition/ReflectionModel/ReflectionComposablePartDefinition.cs

        {
            if (this.IsGeneric())
            {
-                singleMatch = null;
+                singleMatch = null!; // TODO2


📝 the [NotNullWhen(...)] annotation on method seems incorrect #Resolved

Ditto #Resolved

src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/LightCompiler.cs

jcouv · 2020-02-11T04:28:57Z

src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/LightCompiler.cs

            {
-                if (d1.Index > d2.Index) return 1;
+                if (d1!.Index > d2!.Index) return 1; // TODO2


📝 IComparer declares that it can compare null inputs, but this implementation doesn't handle. #Resolved

It's only ever used with non-null values: https://source.dot.net/#System.Linq.Expressions/System/Linq/Expressions/Interpreter/LightCompiler.cs,233 #Resolved

jcouv · 2020-02-11T04:31:21Z

src/libraries/System.Private.CoreLib/src/System/Diagnostics/DebugProvider.cs

@@ -25,7 +28,9 @@ public virtual void Fail(string? message, string? detailMessage)
            }
            WriteAssert(stackTrace, message, detailMessage);
            FailCore(stackTrace, message, detailMessage, "Assertion failed.");
+#nullable disable


📝 without this suppression, the compiler points out that this method could return normally. Looking at the implementation of FailCore, I think it has the same problem.
We need to discuss what is the best way to suppress this warning. Maybe a more fined-grained suppression on a specific diagnostic would be better than the coarse #nullable disable I used... #Resolved

You're nullable disabling/enabling around the closing brace?

Regardless, yes, it's possible for someone to implement Fail in a way that still returns, but that goes against the purpose of the method. If we held true to the possibility that someone did that, then we'd need to remove [DoesNotReturnIf(false)] from Debug.Assert and all such methods, which would be terrible. #Resolved

Can't we add DoesNotReturn to FailCore? #Resolved

@safern FailCore has return statements, so the same warning "method should not return" would be produced there if we add [DoesNotReturn]. #Resolved

jcouv · 2020-02-11T04:32:55Z

...m.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/General/RoAssemblyName.cs

        {
-            if (Name != other.Name)
+            if (Name != other!.Name) // TODO2


📝 not sure what to do here #Resolved

why did we need to make it nullable? #Resolved

This is an internal type, and presumably it's not expecting Equals to be used with null, even though it implements an interface that allows it. Seems like the right change is to remove this ! and ? and suppress the warning on the Equals method signature. #Resolved

To implement IEquatable you must be compatible with the signature bool Equals([AllowNull] T other); #Resolved

To implement IEquatable you must be compatible with the signature bool Equals([AllowNull] T other);

I understand that. So I'm saying use #pragma warning disable whatever to suppress the warning. #Resolved

src/libraries/System.Runtime.Extensions/ref/System.Runtime.Extensions.cs

stephentoub · 2020-02-11T17:36:30Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -639,7 +639,7 @@ private static bool IsSupersetOf(IEnumerable<T> other, MutationInput origin)
        private static MutationResult Add(T item, MutationInput origin)
        {
            OperationResult result;
-            int hashCode = origin.EqualityComparer.GetHashCode(item);
+            int hashCode = origin.EqualityComparer.GetHashCode(item!);


But now it seems like that would actually not just be for TryGetValue, but really all operations on the hash set, at which point we'd be better off using a notnull constraint? Maybe instead of changing the annotations, open an issue about it and keep the ! changes you have here. #Resolved

(And actually, the right answer is probably to do a null check prior to calling GetHashCode, as we do in HashSet<T>.) #Resolved

safern · 2020-02-11T17:41:27Z

src/libraries/Common/src/System/Collections/Generic/ReferenceEqualityComparer.cs

@@ -15,7 +16,7 @@ private ReferenceEqualityComparer()
        {
        }

-        public bool Equals(T x, T y)
+        public bool Equals([AllowNull] T x, [AllowNull] T y)


Since T is constraint to class, can we instead annotate T as T? ? #Resolved

Indeed. I'd missed that. Thanks #Resolved

Hum, it looks like this file is compiled into more than one project. One with nullability enabled and one without. Reverted to use the attribute. #Resolved

Can you use the annotation and just add #nullable enable at the top of the file as well? That's what we've done in other cases like this, where we've not yet gotten to every project that includes it. #Resolved

...braries/System.Linq.Parallel/src/System/Linq/Parallel/QueryOperators/Unary/ForAllOperator.cs

stephentoub · 2020-02-11T17:52:23Z

src/libraries/System.Private.CoreLib/src/System/HashCode.cs

@@ -303,7 +303,8 @@ public void Add<T>(T value)

        public void Add<T>(T value, IEqualityComparer<T>? comparer)
        {
-            Add(comparer != null ? comparer.GetHashCode(value) : (value?.GetHashCode() ?? 0));
+            // TODO2
+            Add(comparer != null ? comparer.GetHashCode(value!) : (value?.GetHashCode() ?? 0));


As with the earlier examples with ImmutableHashSet, we should probably fix this implementation. e.g. today if you wrote code like:

#nullable enable using System; class Program { static void Main(string[] args) { HashCode c = default; c.Add(null, StringComparer.Ordinal); } }

it will blow up. Seems like this line should instead be:

Add(value is null ? 0 : comparer != null ? comparer.GetHashCode(value) : value.GetHashCode());

or something like that. #Resolved

stephentoub · 2020-02-11T17:56:45Z

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs

@@ -307,7 +307,7 @@ public static bool TryGetArray<T>(ReadOnlyMemory<T> memory, out ArraySegment<T>
        {
            TManager? localManager; // Use register for null comparison rather than byref
            manager = localManager = memory.GetObjectStartLength(out _, out _) as TManager;
-            return localManager != null;
+            return localManager != null; // TODO2


What's wrong here? #Resolved

The compiler doesn't know that manager == localManager, so it looks like we may be returning with a null manager when this method returns true.
We could fix that by doing return manager != null;, but that defeats the purpose indicated by above comment ("Use register for null comparison rather than byref"). #Closed

Ah, so this TODO was about improving the compiler :)

For the purposes of this change, just suppress the warning with whatever syntax is least obtrusive. #Resolved

stephentoub · 2020-02-11T17:57:08Z

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs

@@ -329,7 +329,7 @@ public static bool TryGetArray<T>(ReadOnlyMemory<T> memory, out ArraySegment<T>

            Debug.Assert(length >= 0);

-            if (localManager == null)
+            if (localManager == null) // TODO2


What's wrong here? #Resolved

Ditto #Closed

stephentoub · 2020-02-13T01:55:49Z

src/libraries/Common/src/System/Collections/Generic/ReferenceEqualityComparer.cs

 using System.Runtime.CompilerServices;
+#nullable enable


Nit: mind moving this above the usings? #Resolved

stephentoub · 2020-02-13T01:56:50Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -221,7 +221,7 @@ public ImmutableHashSet<T> Remove(T item)
        [Pure]
        public bool TryGetValue(T equalValue, out T actualValue)
        {
-            int hashCode = _equalityComparer.GetHashCode(equalValue);
+            int hashCode = equalValue is object ? _equalityComparer.GetHashCode(equalValue) : 0;


We've generally not used "is object" as a null check.

Suggested change

int hashCode = equalValue is object ? _equalityComparer.GetHashCode(equalValue) : 0;

int hashCode = equalValue != null ? _equalityComparer.GetHashCode(equalValue) : 0;

``` #Resolved

stephentoub · 2020-02-13T01:57:03Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -639,7 +639,7 @@ private static bool IsSupersetOf(IEnumerable<T> other, MutationInput origin)
        private static MutationResult Add(T item, MutationInput origin)
        {
            OperationResult result;
-            int hashCode = origin.EqualityComparer.GetHashCode(item);
+            int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;


Suggested change

int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;

int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;

``` #Resolved

stephentoub · 2020-02-13T01:57:20Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -658,7 +658,7 @@ private static MutationResult Add(T item, MutationInput origin)
        private static MutationResult Remove(T item, MutationInput origin)
        {
            var result = OperationResult.NoChangeRequired;
-            int hashCode = origin.EqualityComparer.GetHashCode(item);
+            int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;


Suggested change

int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;

int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;

``` #Resolved

stephentoub · 2020-02-13T01:57:33Z

...ibraries/System.Collections.Immutable/src/System/Collections/Immutable/ImmutableHashSet_1.cs

@@ -680,7 +680,7 @@ private static MutationResult Remove(T item, MutationInput origin)
        /// </summary>
        private static bool Contains(T item, MutationInput origin)
        {
-            int hashCode = origin.EqualityComparer.GetHashCode(item);
+            int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;


Suggested change

int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;

int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;

``` #Resolved

stephentoub · 2020-02-13T02:02:57Z

...s/System.ComponentModel.Composition/src/Microsoft/Internal/Collections/CollectionServices.cs

        {
            if (second == null)
            {
+                Debug.Assert(value is object);


How do we know this assert is valid? #Resolved

The two callers get their values from ComposablePartDefinition.TryGetExports. That method can return with

both singleMatch and multipleMatches being null (return false;, but then FastAppendToListAllowNulls won't be called because of calling pattern, copied below),

or multipleMatches being set and returning true,

or singleMatch being set and returning true.

singleMatch corresponds to parameter value and multipleMatches corresponds to parameter second here.

Calling pattern:

if (part.TryGetExports(definition, out Tuple<ComposablePartDefinition, ExportDefinition>? singleMatch, out IEnumerable<Tuple<ComposablePartDefinition, ExportDefinition>>? multipleMatches)) { exports = exports.FastAppendToListAllowNulls(singleMatch, multipleMatches); }

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

stephentoub · 2020-02-13T02:03:35Z

src/libraries/System.Linq.Expressions/src/System/Dynamic/Utils/CollectionExtensions.cs

@@ -86,7 +86,10 @@ public static int ListHashCode<T>(this ReadOnlyCollection<T> list)
            int h = 6551;
            foreach (T t in list)
            {
-                h ^= (h << 5) ^ cmp.GetHashCode(t);
+                if (t is object)


Suggested change

if (t is object)

if (t != null)

``` #Resolved

stephentoub · 2020-02-13T02:04:06Z

src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/LightCompiler.cs

            {
+                Debug.Assert(d1 is object && d2 is object);


Suggested change

Debug.Assert(d1 is object && d2 is object);

Debug.Assert(d1 != null && d2 != null);

``` #Resolved

stephentoub · 2020-02-13T02:05:12Z

src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Utils/WrapperEqualityComparer.cs

@@ -43,7 +43,8 @@ public bool Equals(Wrapper<T> x, Wrapper<T> y)
        public int GetHashCode(Wrapper<T> x)
        {
            Debug.Assert(_comparer != null);
-            return _comparer.GetHashCode(x.Value);
+            T value = x.Value;
+            return value is null ? 0 : _comparer.GetHashCode(value);


Suggested change

return value is null ? 0 : _comparer.GetHashCode(value);

return value != null ? 0 : _comparer.GetHashCode(value);

``` #Resolved

src/libraries/System.Private.CoreLib/src/System/Action.cs

This reverts commit f7175ba.

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs

ahsonkhan · 2020-02-14T04:07:37Z

src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.Serialization.cs

@@ -19,7 +19,6 @@ public static void ThrowArgumentException_DeserializeWrongType(Type type, object
            throw new ArgumentException(SR.Format(SR.DeserializeWrongType, type, value.GetType()));
        }

-        [DoesNotReturn]


I am curious, how did you catch this typo?

A new version of the compiler checks method bodies to see that they honor the nullability attributes that they declare in their API. This method has return statements, so it was flagged ;-)

jcouv · 2020-02-14T13:56:31Z

I believe this is ready to merge/squash if I can get the required sign-off(s). Thanks

stephentoub · 2020-02-14T16:12:46Z

Thanks, Julien.

jcouv · 2020-02-14T17:43:29Z

Thanks!

jcouv added 2 commits February 10, 2020 19:04

Help compiler enforce nullability annotations

ae5f683

Revert repro changes

cf34100

jcouv mentioned this pull request Feb 11, 2020

Report bad tightening/loosening of attributes in OHI dotnet/roslyn#41336

Merged

jcouv commented Feb 11, 2020

View reviewed changes

src/libraries/System.Linq.Expressions/src/System/Linq/Expressions/Interpreter/LightCompiler.cs Show resolved Hide resolved

jcouv commented Feb 11, 2020

View reviewed changes

src/libraries/System.Runtime.Extensions/ref/System.Runtime.Extensions.cs Show resolved Hide resolved

Simplifications

1282234

stephentoub reviewed Feb 11, 2020

View reviewed changes

safern reviewed Feb 11, 2020

View reviewed changes

stephentoub reviewed Feb 11, 2020

View reviewed changes

...braries/System.Linq.Parallel/src/System/Linq/Parallel/QueryOperators/Unary/ForAllOperator.cs Show resolved Hide resolved

stephentoub reviewed Feb 11, 2020

View reviewed changes

jcouv added 6 commits February 11, 2020 11:46

Address feedback

d65f2d7

Tweak

3e5405f

Another cleanup

4ee4cea

Fixup on ImportTypes

011b7b7

Revert from T? to [AllowNull]T

3e6f259

Enable nullability on one file

ca6adaa

safern mentioned this pull request Feb 12, 2020

[master] Update dependencies from dotnet/arcade #32115

Merged

This was referenced Feb 12, 2020

System.Net.NetworkInformation.Functional.Tests failing CI and PR #32179

Closed

Test failure: System.Net.NetworkInformation.Tests.NetworkInterfaceBasicTest/BasicTest_AccessInstanceProperties_NoExceptions_Linux #18090

Closed

ahsonkhan mentioned this pull request Feb 12, 2020

Fix up some nullability annotations to remove unnecessary null-forgiving operations (!) #32186

Merged

stephentoub reviewed Feb 13, 2020

View reviewed changes

jcouv added 3 commits February 13, 2020 15:32

Addressing PR feedback from Stephen

b25040f

Use Debug.Assert instead of pragma

f7175ba

Revert "Use Debug.Assert instead of pragma"

1feaf72

This reverts commit f7175ba.

jcouv commented Feb 13, 2020

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/MemoryMarshal.cs Show resolved Hide resolved

jcouv marked this pull request as ready for review February 14, 2020 00:47

jcouv requested review from ahsonkhan, akoeplinger, EgorBo, jozkee, layomia, marek-safar, steveharter, steveisok and vargaz as code owners February 14, 2020 00:47

jcouv mentioned this pull request Feb 14, 2020

Should nullability attributes affect method bodies and OHI? dotnet/roslyn#36039

Closed

3 tasks

ahsonkhan reviewed Feb 14, 2020

View reviewed changes

stephentoub approved these changes Feb 14, 2020

View reviewed changes

stephentoub merged commit 0f834db into dotnet:master Feb 14, 2020

jcouv deleted the nullable-ohi branch February 14, 2020 17:43

jcouv self-assigned this Feb 14, 2020

jcouv added this to the 5.0 milestone Feb 14, 2020

KalleOlaviNiemitalo mentioned this pull request Jul 24, 2020

Publish Microsoft.Bcl.HashCode supporting HashCode.Add(null, comparer) #39895

Open

ghost locked as resolved and limited conversation to collaborators Dec 10, 2020

	int hashCode = equalValue is object ? _equalityComparer.GetHashCode(equalValue) : 0;
	int hashCode = equalValue != null ? _equalityComparer.GetHashCode(equalValue) : 0;
	``` #Resolved

	int hashCode = item is object ? origin.EqualityComparer.GetHashCode(item) : 0;
	int hashCode = item != null ? origin.EqualityComparer.GetHashCode(item) : 0;
	``` #Resolved

	Debug.Assert(d1 is object && d2 is object);
	Debug.Assert(d1 != null && d2 != null);
	``` #Resolved

	return value is null ? 0 : _comparer.GetHashCode(value);
	return value != null ? 0 : _comparer.GetHashCode(value);
	``` #Resolved

Help compiler enforce nullability annotations #32090

Help compiler enforce nullability annotations #32090

Conversation

jcouv commented Feb 11, 2020 • edited Loading

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

safern Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

safern Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

safern Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

jcouv Feb 12, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 11, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 11, 2020 • edited Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

stephentoub Feb 13, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Feb 13, 2020

Choose a reason for hiding this comment

jcouv commented Feb 11, 2020 •

edited

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

safern Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

jcouv Feb 11, 2020 •

edited

Loading

safern Feb 11, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

safern Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

jcouv Feb 12, 2020 •

edited

Loading

stephentoub Feb 12, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

stephentoub Feb 11, 2020 •

edited by jcouv

Loading

jcouv Feb 11, 2020 •

edited

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading

stephentoub Feb 13, 2020 •

edited by jcouv

Loading