Use lightweight type constraint binding for IsValueType #48270

jcouv · 2020-10-03T02:17:48Z

~~Fixes #41368 by building up on #47968~~

When binding type constraints for purpose of computing IsValueType, we don't need to bind the type arguments of the type constraints. We can bind the type constraints by using placeholder type arguments.
This allows making a fix to nullability rules for type substitution (also included in this PR), which we'd previously held off because of cycles.

We'll discuss whether this is better to take for 16.8 or 16.9. As the bootstrapping issue shows, the fix catches some nullability mismatches that were previously unreported, so it may be better to keep this change in early 16.9 so that VS and BCL can adjust accordingly (if needed).

Overview:

the central change is in BindGenericSimpleNamespaceOrTypeOrAliasSymbol in Binder_Symbols.cs
the fix for the substitution bug is in TypeWithAnnotations.SubstituteTypeCore
changed how we store _lazyTypeParameterConstraints (in NamedTypeSymbol and other) and _lazyBounds (in TypeParameterSymbol) is because bad constraints get filtered out. Otherwise, we had a bug where we were reporting diagnostics in the wrong phase (based on result of lightweight binding instead of full binding).

Note: there is a bootstrapping issue. The fix reveals some wrong annotations in Roslyn, but fixing them is a bit of a rabbit hole. Still working on that.

Update: putting this PR on hold for a short while. We think the nullability bug probably shouldn't be fixed by changing type substitution. So I'll take that part out of this PR.

CyrusNajmabadi · 2020-10-05T05:05:34Z

...Workspaces/SharedUtilitiesAndExtensions/Workspace/CSharp/Extensions/ITypeSymbolExtensions.cs

-            return typeSymbol.Accept(ExpressionSyntaxGeneratorVisitor.Instance).WithAdditionalAnnotations(Simplifier.Annotation);
+            var expression = typeSymbol.Accept(ExpressionSyntaxGeneratorVisitor.Instance);
+            RoslynDebug.Assert(expression is not null);
+            return expression.WithAdditionalAnnotations(Simplifier.Annotation);


what happened here that needed this?

@CyrusNajmabadi @tmat Please hold off on reviewing the IDE portion (it's not complete yet). I'll ping once the compiler portion is reviewed. Thanks
In short, ISerializerService.Deserialize is missing a ? annotation on the return type, and there are ripples from that. #Closed

CyrusNajmabadi · 2020-10-05T05:06:03Z

src/Workspaces/Core/Portable/Remote/ISerializerService.cs

@@ -30,7 +30,7 @@ internal interface ISerializerService : IWorkspaceService

        void SerializeOptionSet(SerializableOptionSet options, ObjectWriter writer, CancellationToken cancellationToken);

-        T Deserialize<T>(WellKnownSynchronizationKind kind, ObjectReader reader, CancellationToken cancellationToken);
+        T? Deserialize<T>(WellKnownSynchronizationKind kind, ObjectReader reader, CancellationToken cancellationToken);


@tmat to confirm OOP serialization changes.

cston · 2020-10-05T19:32:46Z

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

+        /// - an actual value, which may be from lightweight or full binding, depending on <see cref="TypeParameterBounds.UsedLightweightTypeConstraintBinding"/>
+        ///
+        /// The distinction between <see cref="TypeParameterBounds.NullFromLightweightBinding"/> and null
+        /// helps us ensure that we do full binding at some point and thus produce diagnostics.


What is an example where we need the distinction? (When would be have null from lightweight binding and produce diagnostics from full binding?) #ByDesign

One scenario where the difference was visible was where T : Span<int>. The current diagnostic complains about Span<int> (and that constraint gets filtered out), but if we report the diagnostic from lightweight binding we'll complain about Span<T>. (see GenericArgsAndConstraints)
It's not a big difference, but also we may add more such scenarios, so I'm not sure we want to chance it. Also, if we decided to keep the previous design, we'd want to add comments, as this was surprising when I had to step through. #ByDesign

cston · 2020-10-05T19:34:12Z

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

@@ -67,7 +67,7 @@ internal partial class Binder
                    Debug.Assert(ordinal < n);

                    (TypeParameterConstraintClause constraintClause, ArrayBuilder<TypeConstraintSyntax>? typeConstraintNodes) =
-                        this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, canIgnoreNullableContext: canIgnoreNullableContext, diagnostics);
+                        this.BindTypeParameterConstraints(typeParameterList.Parameters[ordinal], clause, isForOverride, canUseLightweightTypeConstraintBinding, diagnostics);


canUseLightweightTypeConstraintBinding [](start = 120, length = 38)

Consider keeping the named argument since there are two bool parameters in a row. #Closed

cston · 2020-10-05T19:34:54Z

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

@@ -119,7 +119,7 @@ internal partial class Binder
        /// Bind and return a single type parameter constraint clause along with syntax nodes corresponding to type constraints.
        /// </summary>
        private (TypeParameterConstraintClause, ArrayBuilder<TypeConstraintSyntax>?) BindTypeParameterConstraints(
-            TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, bool canIgnoreNullableContext, DiagnosticBag diagnostics)
+            TypeParameterSyntax typeParameterSyntax, TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, bool canUseLightweightTypeConstraintBinding, DiagnosticBag diagnostics)


canUseLightweightTypeConstraintBinding [](start = 138, length = 38)

Argument is not used. #Closed

cston · 2020-10-05T19:39:58Z

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

@@ -1126,7 +1126,11 @@ private Symbol UnwrapAlias(Symbol symbol, out AliasSymbol alias, DiagnosticBag d
                diagnostics, basesBeingResolved, qualifierOpt, node, plainName, node.Arity, options);
            NamedTypeSymbol resultType;

-            if (isUnboundTypeExpr)
+            if ((Flags & BinderFlags.LightweightTypeConstraintBinding) != 0)


(Flags & BinderFlags.LightweightTypeConstraintBinding) != 0 [](start = 16, length = 59)

Should this check be moved to the else block (after if (isUnboundTypeExpr))? #Closed

cston · 2020-10-05T19:41:50Z

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

@@ -34,14 +38,20 @@ internal sealed class TypeParameterBounds
            this.Interfaces = interfaces;
            this.EffectiveBaseClass = effectiveBaseClass;
            this.DeducedBaseType = deducedBaseType;
-            this.IgnoresNullableContext = ignoresNullableContext;
+            this.UsedLightweightTypeConstraintBinding = usedLightweightTypeConstraintBinding;
        }

        private TypeParameterBounds()
        {
        }


Consider removing this constructor. #Closed

It may be silly, but I didn't want to call passing an explicit usedLightweightTypeConstraintBinding: false value for the Unset constant, given that it has a very different meaning. #Closed

cston · 2020-10-05T19:45:34Z

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

@@ -60,9 +60,9 @@ public static ImmutableArray<TypeWithAnnotations> CreateTypeArguments(ImmutableA
        public static readonly ErrorTypeSymbol Instance = new UnboundArgumentErrorTypeSymbol(string.Empty, new CSDiagnosticInfo(ErrorCode.ERR_UnexpectedUnboundGenericName));

        private readonly string _name;
-        private readonly DiagnosticInfo _errorInfo;
+        private readonly DiagnosticInfo? _errorInfo;


DiagnosticInfo? [](start = 25, length = 15)

Is this change necessary? #Closed

The change is correct (some instances are created with null) but not necessary. I'll undo so this PR is more focused. #Resolved

cston · 2020-10-05T19:49:41Z

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

+    /// we'll just use these placeholders instead. That's good enough binding to compute
+    /// <see cref="TypeSymbol.IsValueType"/> with minimal binding.
+    /// </summary>
+    internal sealed class PlaceholderTypeArgumentSymbol : ErrorTypeSymbol


PlaceholderTypeArgumentSymbol [](start = 26, length = 29)

Could we use UnboundArgumentErrorTypeSymbol instead? #ByDesign

We could (and I did initially), but I felt that seeing a distinct type would be helpful to someone who ever sees this when debugging. It's clearly its own thing (I think having a distinct type is good).
I could try and share more code though... #ByDesign

333fred · 2020-10-09T18:14:04Z

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

        {
-            if (!_lazyTypeParameterConstraints.HasValue(canIgnoreNullableContext))
+            // We're not using lightweight type constraint binding for local function because the risk of cycles is minimal (no overrides)


Man does this feel like a statement that's someone is going to look at in a few years and go "well that was wrong" #Resolved

I also prefer us not having this difference by comparison to other type parameters.

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

333fred · 2020-10-09T18:15:26Z

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

@@ -253,25 +255,27 @@ private ImmutableArray<TypeParameterSymbol> MakeTypeParameters(DiagnosticBag dia
        /// <summary>
        /// Returns the constraint clause for the given type parameter.
        /// </summary>
-        internal TypeParameterConstraintClause GetTypeParameterConstraintClause(bool canIgnoreNullableContext, int ordinal)
+        internal TypeParameterConstraintClause GetTypeParameterConstraintClause(bool canUseLightweightTypeConstraintBinding, int ordinal)


Consider nullable enabling this code. #Resolved

333fred · 2020-10-09T18:24:03Z

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

@@ -55,6 +56,33 @@ internal enum TypeParameterConstraintKind
        AllNonNullableKinds = ReferenceType | ValueType | Constructor | Unmanaged,
    }

+    internal sealed class TypeParameterConstraintClauses


Consider nullable enabling this. #Resolved

jcouv · 2020-10-09T19:20:08Z

@cston Based on feedback from @AlekseyTs, I'm going to take a stab at simplifying the two-level caches. We'll go back to only caching the fully-bound constraints. IsValueType and IsReferenceType can be cached separately with less risk.
Let me know if you have a concern with that approach. #Closed

cston · 2020-10-09T19:31:33Z

@cston Based on feedback from Aleksey, I'm going to take a stab at simplifying the two-level caches. We'll go back to only caching the fully-bound constraints. IsValueType and IsReferenceType can be cached separately with less risk.

Is there a reason to make that change in this PR? We're already binding with two levels based on canIgnoreNullableContext, so the change in this PR is limited to just those differences from binding type arguments.

If we want to change caching of IsValueType and IsReferenceType, that should be handled in a subsequent PR. #Closed

jcouv · 2020-10-09T19:33:59Z

I'm fine to keep that for a subsequent PR. @AlekseyTs Would that be okay with you? #Closed

AlekseyTs · 2020-10-10T03:36:09Z

Would that be okay with you?

I'd prefer reviewing one PR rather than two. Is the caching change going to be very disruptive? If it will be in a separate commit, it will be easy to review incrementally, if that is the reason. #Closed

AlekseyTs · 2020-10-12T13:51:38Z

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

        /// </summary>
-        IgnoreNullableContext = 1u << 31,
+        LightweightTypeConstraintBinding = 1u << 31,


LightweightTypeConstraintBinding [](start = 8, length = 32)

It looks like this flag is used in a function that is not specific to constraint binding, but rather it is our core code path to bind types in general. Let's keep the name of this flag indicating what that function should do, instead of reflecting mainly why. I suggest "SuppressTypeArgumentBinding", or something like that. #Resolved

AlekseyTs · 2020-10-12T13:53:57Z

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

@@ -113,9 +113,9 @@ internal enum BinderFlags : uint
        InEEMethodBinder = 1 << 30,

        /// <summary>
-        /// Assume '#nullable disable' context.
+        /// Skip binding type arguments in type constraints


in type constraints [](start = 39, length = 20)

"Skip binding type arguments. For example, currently used when type constraints are bound in some scenarios."?
Also, consider describing what is used for type arguments when this flag is set. #Resolved

AlekseyTs · 2020-10-12T14:06:11Z

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

@@ -90,6 +90,14 @@ internal static class ConstraintsHelper
            }

            diagnosticsBuilder.Free();
+
+            if (bounds is null && canUseLightweightTypeConstraintBinding)


if (bounds is null && canUseLightweightTypeConstraintBinding) [](start = 12, length = 61)

Would we need this additional logic if we never cached "light-bound" constraints on a symbol?

AlekseyTs · 2020-10-12T14:13:07Z

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

@@ -55,6 +56,33 @@ internal enum TypeParameterConstraintKind
        AllNonNullableKinds = ReferenceType | ValueType | Constructor | Unmanaged,
    }

+    internal sealed class TypeParameterConstraintClauses


TypeParameterConstraintClauses [](start = 26, length = 30)

Would we need this type if we never cached "light-bound" constraints on a symbol? I would prefer us to never add it if we could do without.

AlekseyTs · 2020-10-12T14:24:31Z

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

@@ -468,6 +468,11 @@ internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
                return newTypeWithModifiers;
            }

+            if (newTypeWithModifiers.Type is PlaceholderTypeArgumentSymbol)


if (newTypeWithModifiers.Type is PlaceholderTypeArgumentSymbol) [](start = 12, length = 63)

Is this special case necessary? #Resolved

Yes, without it we run into the following cycle:

Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol.IsValueType.get() Line 570 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbolExtensions.IsTypeParameterDisallowingAnnotationInCSharp8(Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol type) Line 68 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.SubstituteTypeCore(Microsoft.CodeAnalysis.CSharp.Symbols.AbstractTypeMap typeMap) Line 499 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.NonLazyType.SubstituteType(Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations type, Microsoft.CodeAnalysis.CSharp.Symbols.AbstractTypeMap typeMap) Line 920 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.SubstituteType(Microsoft.CodeAnalysis.CSharp.Symbols.AbstractTypeMap typeMap) Line 434 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.AbstractTypeMap.SubstituteNamedType(Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol previous) Line 68 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.AbstractTypeMap.SubstituteNamedTypes(System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol> original) Line 416 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SubstitutedNamedTypeSymbol.InterfacesNoUseSiteDiagnostics(Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved) Line 154 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.GetInterfaceInfo() Line 107 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.GetAllInterfaces() Line 323 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.AllInterfacesNoUseSiteDiagnostics.get() Line 206 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol.AllInterfacesWithDefinitionUseSiteDiagnostics(ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 212 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.GetBaseInterfaces(Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol type, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 971 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupMembersInInterfaceOnly(Microsoft.CodeAnalysis.CSharp.LookupResult current, Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol type, string name, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, Microsoft.CodeAnalysis.CSharp.Binder originalBinder, Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol accessThroughType, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 962 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupMembersInInterface(Microsoft.CodeAnalysis.CSharp.LookupResult current, Microsoft.CodeAnalysis.CSharp.Symbols.NamedTypeSymbol type, string name, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, Microsoft.CodeAnalysis.CSharp.Binder originalBinder, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 1077 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupMembersInType(Microsoft.CodeAnalysis.CSharp.LookupResult result, Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol type, string name, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, Microsoft.CodeAnalysis.CSharp.Binder originalBinder, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 188 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupMembersInternal(Microsoft.CodeAnalysis.CSharp.LookupResult result, Microsoft.CodeAnalysis.CSharp.Symbols.NamespaceOrTypeSymbol nsOrType, string name, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, Microsoft.CodeAnalysis.CSharp.Binder originalBinder, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 174 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupSymbolsOrMembersInternal(Microsoft.CodeAnalysis.CSharp.LookupResult result, Microsoft.CodeAnalysis.CSharp.Symbols.NamespaceOrTypeSymbol qualifierOpt, string name, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 140 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.LookupSymbolsSimpleName(Microsoft.CodeAnalysis.CSharp.LookupResult result, Microsoft.CodeAnalysis.CSharp.Symbols.NamespaceOrTypeSymbol qualifierOpt, string plainName, int arity, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, Microsoft.CodeAnalysis.CSharp.LookupOptions options, bool diagnose, ref System.Collections.Generic.HashSet<Microsoft.CodeAnalysis.DiagnosticInfo> useSiteDiagnostics) Line 42 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindNonGenericSimpleNamespaceOrTypeOrAliasSymbol(Microsoft.CodeAnalysis.CSharp.Syntax.IdentifierNameSyntax node, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics, Microsoft.CodeAnalysis.CSharp.Symbols.NamespaceOrTypeSymbol qualifierOpt) Line 871 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindSimpleNamespaceOrTypeOrAliasSymbol(Microsoft.CodeAnalysis.CSharp.Syntax.SimpleNameSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics, Microsoft.CodeAnalysis.CSharp.Symbols.NamespaceOrTypeSymbol qualifierOpt) Line 810 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindQualifiedName(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax leftName, Microsoft.CodeAnalysis.CSharp.Syntax.SimpleNameSyntax rightName, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics) Line 1418 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindNamespaceOrTypeOrAliasSymbol(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics) Line 423 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrAlias(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics) Line 317 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrAliasOrConstraintKeyword(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, out Microsoft.CodeAnalysis.CSharp.Binder.ConstraintContextualKeyword keyword) Line 177 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrConstraintKeyword(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, out Microsoft.CodeAnalysis.CSharp.Binder.ConstraintContextualKeyword keyword) Line 59 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeParameterConstraints(Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterSyntax typeParameterSyntax, Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) Line 261 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeParameterConstraintClauses(Microsoft.CodeAnalysis.CSharp.Symbol containingSymbol, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol> typeParameters, Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterListSyntax typeParameterList, Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterConstraintClauseSyntax> clauses, ref System.Collections.Generic.IReadOnlyDictionary<Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol, bool> isValueTypeOverride, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, bool isForOverride) Line 68 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceNamedTypeSymbol.MakeTypeParameterConstraints(bool canUseLightweightTypeConstraintBinding, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) Line 333 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceNamedTypeSymbol.GetTypeParameterConstraintClause(bool canUseLightweightTypeConstraintBinding, int ordinal) Line 265 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceTypeParameterSymbol.GetTypeParameterConstraintClause(bool canUseLightweightTypeConstraintBinding) Line 573 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceTypeParameterSymbol.GetDeclaredConstraints(bool canUseLightweightTypeConstraintBinding) Line 590 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceTypeParameterSymbol.HasValueTypeConstraint.get() Line 513 C# Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol.IsValueType.get() Line 570 C#

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

AlekseyTs · 2020-10-12T14:36:32Z

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

                }
                else
                {
-                    newAnnotation = newTypeWithModifiers.NullableAnnotation;
+                    newAnnotation = NullableAnnotation;


newAnnotation = NullableAnnotation; [](start = 20, length = 35)

If new annotation is oblivious, shouldn't that always win for typeSymbol.IsTypeParameterDisallowingAnnotationInCSharp8()?

Trying to answer this question, I tried to spec out the behavior of this method to make sure I understand it.

I wrote the matrix that I think we want: https://gist.github.com/jcouv/4dd5291e65a4cbffa83c03440427d557
Took a look with Chuck and made some small adjustments.
I would summarize this matrix as: (1) annotated original or substitution type yields annotated result, (2) otherwise, use the annotation from substitution type.

@AlekseyTs, does that look right to you too? I've included a legend to help decode that matrix, but would be happy to hop on a call to walk you through.

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

AlekseyTs · 2020-10-12T14:42:16Z

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

@@ -489,18 +494,18 @@ internal TypeWithAnnotations SubstituteTypeCore(AbstractTypeMap typeMap)
            {
                newAnnotation = NullableAnnotation;
            }
-            else if (NullableAnnotation != NullableAnnotation.Oblivious)
+            else if (NullableAnnotation.IsNotAnnotated())


else if (NullableAnnotation.IsNotAnnotated()) [](start = 12, length = 45)

Would it make sense to add a check if old and new annotations are equal, right before this condition?

AlekseyTs · 2020-10-12T14:44:29Z

    private sealed class LazyNullableTypeParameter : Extensions

Do we still need this machinery? #ByDesign

Refers to: src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs:937 in 618defa. [](commit_id = 618defa, deletion_comment = False)

AlekseyTs · 2020-10-12T14:47:26Z

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

+
+        public static ImmutableArray<TypeWithAnnotations> CreateTypeArguments(ImmutableArray<TypeParameterSymbol> typeParameters)
+        {
+            return typeParameters.SelectAsArray(_ => s_instance);


SelectAsArray [](start = 34, length = 13)

Can we add a Repeate helper instead? All we are using from typeParameters is length.

We could, but I changed the code from populating an ArrayBuilder to using SelectAsArray in previous feedback. I don't think this is worth changing back again.

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

I changed the code from populating an ArrayBuilder to using SelectAsArray in previous feedback. I don't think this is worth changing back again.

This isn't exactly what I am suggesting.

In reply to: 503602627 [](ancestors = 503602627,503346990)

AlekseyTs · 2020-10-12T14:49:13Z

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

+            }
+        }
+
+        internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt = null)


Equals [](start = 31, length = 6)

Should we override GetHashCode? #Pending

I removed it based on earlier feedback.

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

I removed it based on earlier feedback.

I think we should have it here.

In reply to: 503602842 [](ancestors = 503602842,503348130)

AlekseyTs · 2020-10-12T14:49:57Z

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

+
+        internal override bool Equals(TypeSymbol t2, TypeCompareKind comparison, IReadOnlyDictionary<TypeParameterSymbol, bool>? isValueTypeOverrideOpt = null)
+        {
+            return (object)t2 == this;


(object)t2 == this [](start = 19, length = 18)

true? #Closed

If we returned true, then this symbol equals every other symbol.

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

If we returned true, then this symbol equals every other symbol.

Right, I was thinking that all instances of this type should be considered equal to each other (since we have only one). But what I suggested clearly doesn't do that and it is probably not worth switching to a type check here.

In reply to: 503622221 [](ancestors = 503622221,503348621)

AlekseyTs · 2020-10-12T15:18:47Z

src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs

@@ -138511,7 +138566,14 @@ class B2 : A2
    public override void F6<T>([DisallowNull] T t) { }
 }";
            comp = CreateCompilation(new[] { sourceB2, DisallowNullAttributeDefinition, NotNullAttributeDefinition }, references: new[] { refA });
-            comp.VerifyDiagnostics();
+            comp.VerifyDiagnostics(
+                // (14,26): warning CS8765: Nullability of type of parameter 't' doesn't match overridden member (possibly because of nullability attributes).


// (14,26): warning CS8765: Nullability of type of parameter 't' doesn't match overridden member (possibly because of nullability attributes). [](start = 16, length = 142)

Is this warning caused by the attribute? Could we add an override without it to observe the fact? #Pending

AlekseyTs · 2020-10-12T15:20:45Z

Done with review pass (iteration 20)

jcouv · 2020-10-13T01:35:07Z

    private sealed class LazyNullableTypeParameter : Extensions

I tried removing it, but then we get into cycles. Example: static void M2<T, U>(U x, U? y) where U : T?

  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol.IsValueType.get() Line 570 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeWithAnnotations.SetIsAnnotated(Microsoft.CodeAnalysis.CSharp.CSharpCompilation compilation) Line 196 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindNamespaceOrTypeOrAliasSymbol.__bindNullable|941_1(ref Microsoft.CodeAnalysis.CSharp.Binder.<>c__DisplayClass941_0 value) Line 522 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindNamespaceOrTypeOrAliasSymbol(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics) Line 406 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrAlias(Microsoft.CodeAnalysis.CSharp.Syntax.ExpressionSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, Roslyn.Utilities.ConsList<Microsoft.CodeAnalysis.CSharp.Symbols.TypeSymbol> basesBeingResolved, bool suppressUseSiteDiagnostics) Line 317 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrAliasOrConstraintKeyword(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, out Microsoft.CodeAnalysis.CSharp.Binder.ConstraintContextualKeyword keyword) Line 177 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeOrConstraintKeyword(Microsoft.CodeAnalysis.CSharp.Syntax.TypeSyntax syntax, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, out Microsoft.CodeAnalysis.CSharp.Binder.ConstraintContextualKeyword keyword) Line 59 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeParameterConstraints(Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterSyntax typeParameterSyntax, Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterConstraintClauseSyntax constraintClauseSyntax, bool isForOverride, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) Line 261 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Binder.BindTypeParameterConstraintClauses(Microsoft.CodeAnalysis.CSharp.Symbol containingSymbol, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol> typeParameters, Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterListSyntax typeParameterList, Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterConstraintClauseSyntax> clauses, ref System.Collections.Generic.IReadOnlyDictionary<Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol, bool> isValueTypeOverride, Microsoft.CodeAnalysis.DiagnosticBag diagnostics, bool isForOverride) Line 68 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.ConstraintsHelper.MakeTypeParameterConstraints(Microsoft.CodeAnalysis.CSharp.Symbol containingSymbol, Microsoft.CodeAnalysis.CSharp.Binder binder, System.Collections.Immutable.ImmutableArray<Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol> typeParameters, Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterListSyntax typeParameterList, Microsoft.CodeAnalysis.SyntaxList<Microsoft.CodeAnalysis.CSharp.Syntax.TypeParameterConstraintClauseSyntax> constraintClauses, bool canUseLightweightTypeConstraintBinding, Microsoft.CodeAnalysis.DiagnosticBag diagnostics) Line 350 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceOrdinaryMethodSymbol.GetTypeParameterConstraintClauses(bool canUseLightweightTypeConstraintBinding) Line 296 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMethodTypeParameterSymbol.GetTypeParameterConstraintClause(bool canUseLightweightTypeConstraintBinding) Line 694 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMethodTypeParameterSymbol.GetDeclaredConstraints(bool canUseLightweightTypeConstraintBinding) Line 712 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.SourceMethodTypeParameterSymbol.HasValueTypeConstraint.get() Line 634 C#
  Microsoft.CodeAnalysis.CSharp.dll!Microsoft.CodeAnalysis.CSharp.Symbols.TypeParameterSymbol.IsValueType.get() Line 570 C#

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

Refers to: src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs:937 in 618defa. [](commit_id = 618defa, deletion_comment = False)

AlekseyTs · 2020-10-13T02:21:46Z

    private sealed class LazyNullableTypeParameter : Extensions

I tried removing it, but then we get into cycles. Example: static void M2<T, U>(U x, U? y) where U : T?

Perhaps we should keep the part that ignores nullable context while constraints are bound?

In reply to: 707430606 [](ancestors = 707430606,707164189)

Refers to: src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs:937 in 618defa. [](commit_id = 618defa, deletion_comment = False)

AlekseyTs · 2020-10-13T17:00:14Z

        TypeParameterConstraintClause.AdjustConstraintTypes(containingSymbol, typeParameters, results, ref isValueTypeOverride);

It looks like here is yet another way to avoid cycles during constraint binding. Is it still needed?

Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs:103 in 2273303. [](commit_id = 2273303, deletion_comment = False)

jcouv · 2020-10-13T17:44:46Z

    private sealed class LazyNullableTypeParameter : Extensions

I thought about that. The stack trace above had that part of the code restored. The problem is that do need information about T to decide what T? means, but to do that we need to bind type constraints. That is still true when we do lightweight binding.

In reply to: 707443434 [](ancestors = 707443434,707430606,707164189)

Refers to: src/Compilers/CSharp/Portable/Symbols/TypeWithAnnotations.cs:937 in 618defa. [](commit_id = 618defa, deletion_comment = False)

jcouv · 2020-10-13T17:51:37Z

        TypeParameterConstraintClause.AdjustConstraintTypes(containingSymbol, typeParameters, results, ref isValueTypeOverride);

Thanks, I'll take a look. That makes me think Equals also takes a dictionary for IsValueType information, I'll experiment to see if it is still needed.

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

Refers to: src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs:103 in 2273303. [](commit_id = 2273303, deletion_comment = False)

jcouv · 2020-11-11T06:16:33Z

Closing. This PR was subsumed by #49261

jcouv self-assigned this Oct 3, 2020

jcouv added 2 commits October 3, 2020 20:53

Use lightweight type constraint binding for IsValueType

4c83255

Rename binder flag

3df16ba

jcouv force-pushed the constraint-cycle branch from 2f12174 to 36e873c Compare October 4, 2020 04:35

jcouv added 6 commits October 4, 2020 09:22

Fix TypeParameterSymbol storage of bounds

5c026fb

Fix phasing in SourceNamedTypeSymbol

0f2136a

Rename

7c55588

Remove flag from TypeParameterConstraintClause

a5ae914

Add test for bootstrap issue

7042f3b

WIP on IDE layer bootstrapping

36cd621

jcouv force-pushed the constraint-cycle branch from 36e873c to 36cd621 Compare October 4, 2020 18:26

Use unreported error code

2dbbbe2

jcouv marked this pull request as ready for review October 5, 2020 03:47

jcouv requested review from a team as code owners October 5, 2020 03:47

jcouv added 0 - Backlog Area-Compilers and removed 0 - Backlog labels Oct 5, 2020

CyrusNajmabadi reviewed Oct 5, 2020

View reviewed changes

jcouv requested a review from cston October 5, 2020 17:24

cston reviewed Oct 5, 2020

View reviewed changes

jcouv added 2 commits October 5, 2020 12:50

Address feedback from Chuck

f24bc69

Revert span test

6d5e7a5

333fred reviewed Oct 9, 2020

View reviewed changes

AlekseyTs reviewed Oct 12, 2020

View reviewed changes

Address feedback from Fred and Aleksey

2273303

jcouv marked this pull request as draft October 14, 2020 03:37

AlekseyTs mentioned this pull request Nov 10, 2020

Avoid cycles while binding type parameter constraints #49261

Merged

jcouv closed this Nov 11, 2020

Use lightweight type constraint binding for IsValueType #48270

Use lightweight type constraint binding for IsValueType #48270

Conversation

jcouv commented Oct 3, 2020 • edited Loading

Choose a reason for hiding this comment

jcouv Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

jcouv Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

jcouv Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

cston Oct 5, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

333fred Oct 9, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

333fred Oct 9, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

333fred Oct 9, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

jcouv commented Oct 9, 2020 • edited Loading

cston commented Oct 9, 2020 • edited by jcouv Loading

jcouv commented Oct 9, 2020 • edited Loading

AlekseyTs commented Oct 10, 2020 • edited by jcouv Loading

AlekseyTs Oct 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

AlekseyTs Oct 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Oct 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs commented Oct 12, 2020 • edited by jcouv Loading

AlekseyTs Oct 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Oct 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Oct 12, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs Oct 12, 2020 • edited by jcouv Loading

Choose a reason for hiding this comment

AlekseyTs commented Oct 12, 2020

jcouv commented Oct 13, 2020

AlekseyTs commented Oct 13, 2020

AlekseyTs commented Oct 13, 2020

jcouv commented Oct 13, 2020

jcouv commented Oct 13, 2020

jcouv commented Nov 11, 2020

jcouv commented Oct 3, 2020 •

edited

Loading

jcouv Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited by jcouv

Loading

jcouv Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited

Loading

jcouv Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited

Loading

jcouv Oct 5, 2020 •

edited

Loading

cston Oct 5, 2020 •

edited by jcouv

Loading

jcouv Oct 5, 2020 •

edited

Loading

333fred Oct 9, 2020 •

edited by jcouv

Loading

333fred Oct 9, 2020 •

edited by jcouv

Loading

333fred Oct 9, 2020 •

edited by jcouv

Loading

jcouv commented Oct 9, 2020 •

edited

Loading

cston commented Oct 9, 2020 •

edited by jcouv

Loading

jcouv commented Oct 9, 2020 •

edited

Loading

AlekseyTs commented Oct 10, 2020 •

edited by jcouv

Loading

AlekseyTs Oct 12, 2020 •

edited by jcouv

Loading

AlekseyTs Oct 12, 2020 •

edited by jcouv

Loading

AlekseyTs Oct 12, 2020 •

edited by jcouv

Loading

AlekseyTs commented Oct 12, 2020 •

edited by jcouv

Loading

AlekseyTs Oct 12, 2020 •

edited

Loading

AlekseyTs Oct 12, 2020 •

edited by jcouv

Loading

AlekseyTs Oct 12, 2020 •

edited

Loading

AlekseyTs Oct 12, 2020 •

edited by jcouv

Loading