Adjust receiver in instance invocations #73501

jcouv · 2024-05-16T00:14:36Z

This PR is on hold pending confirmation on the Unsafe.As approach

Relates to test plan #66722

jjonescz

(haven't looked at tests yet)

jjonescz · 2024-05-16T09:58:06Z

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

+            }
+            else
+            {
+                throw ExceptionUtilities.UnexpectedValue(receiver.Type);


Is this going to throw for a receiver of unconstrained generic parameter type? #Resolved

We currently don't perform extension member lookup on type parameters. When we do (planned), this section will need to be expanded.

jjonescz · 2024-05-16T10:03:57Z

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

@@ -108,6 +108,8 @@ private BoundExpression VisitIndexerAccess(BoundIndexerAccess node, bool isLeftO
            BoundIndexerAccess? oldNodeOpt,
            bool isLeftOfAssignment)
        {
+            rewrittenReceiver = AdjustReceiverForExtensionsIfNeeded(rewrittenReceiver, indexer);


We should probably have tests that VisitArgumentsAndCaptureReceiverIfNeeded behaves correctly with our rewritten receiver. #Resolved

There was a problem with interpolation handler arguments (fixed by adjusting the type of the interpolation "instance" placeholder). Thanks

jjonescz · 2024-05-16T10:05:30Z

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

+            }
+            else if (receiver.Type.IsValueType)
+            {
+                receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);


Consider testing that this ref temp is going to report errors correctly in async methods for example. #Resolved

AlekseyTs · 2024-05-16T15:09:53Z

@jcouv It looks like there are legitimate test failures

AlekseyTs · 2024-05-16T15:27:01Z

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

@@ -380,6 +379,8 @@ BoundExpression visitArgumentsAndFinishRewrite(BoundCall node, BoundExpression?
                    rewrittenReceiver = null;
                }

+                rewrittenReceiver = AdjustReceiverForExtensionsIfNeeded(rewrittenReceiver, method);


AdjustReceiverForExtensionsIfNeeded

I think that whenever possible, this transformation should be done in the same code that uses ReferToTempIfReferenceTypeReceiver. It looks like for this specific code path it will be possible. Basically, there should be one place where we transform receivers for a given language costruct. #Closed

AlekseyTs · 2024-05-16T16:10:35Z

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

+                {
+                    var thisExtensionType = (SourceExtensionTypeSymbol)receiver.Type;
+                    Debug.Assert(thisExtensionType.UnderlyingInstanceField is not null);
+                    Debug.Assert(thisExtensionType.UnderlyingInstanceField.Type.Equals(member.ContainingType, TypeCompareKind.AllIgnoreOptions));


thisExtensionType.UnderlyingInstanceField.Type.Equals(member.ContainingType, TypeCompareKind.AllIgnoreOptions)

I think this should be a strong check rather than an assert. If this condition is false, there is something wrong and we cannot continue. #Closed

AlekseyTs · 2024-05-16T16:14:22Z

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

+                    Debug.Assert(thisExtensionType.UnderlyingInstanceField is not null);
+                    Debug.Assert(thisExtensionType.UnderlyingInstanceField.Type.Equals(member.ContainingType, TypeCompareKind.AllIgnoreOptions));
+
+                    return _factory.Field(receiver, thisExtensionType.UnderlyingInstanceField);


return _factory.Field(receiver, thisExtensionType.UnderlyingInstanceField);

It is not obvious when this transformation should happen when interpolation handlers are involved. A PROTOTYPE comment should be good enough for now. #Closed

AlekseyTs · 2024-05-16T16:18:57Z

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

+            }
+            else if (receiver.Type.IsValueType)
+            {
+                receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);


RefKind.Ref

We probably should extract ref kind from receiver, see how VisitArgumentsAndCaptureReceiverIfNeeded figures out what ref kind to use. I think we should share the logic. #Closed

I didn't follow this. VisitArgumentsAndCaptureReceiverIfNeeded saves the receiver into a temp when we need multiple re-uses of the receiver (either because we're doing some compound/complex operation or we'll need the receiver a second time as part of an interpolation handler).
But the purpose of the temp here is not multiple re-uses.

I didn't follow this.

Let's sync up offline

AlekseyTs · 2024-05-16T16:21:31Z

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

+            }
+            else if (receiver.Type.IsValueType)
+            {
+                receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);


MakeTemp

I would prefer if we used _factory.StoreToTemp explicitly as VisitArgumentsAndCaptureReceiverIfNeeded does. It is easier to follow the code when we rely on the same helpers when we are doing the same things. #Closed

AlekseyTs · 2024-05-16T16:26:35Z

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

+
+            var call = _factory.Call(null,
+                            _factory.WellKnownMethod(WellKnownMember.System_Runtime_CompilerServices_Unsafe__As_T)
+                                .Construct(ImmutableArray.Create<TypeSymbol>(receiver.Type, memberExtensionType)),


memberExtensionType

Is there a guarantee that this type is going to satisfy the constraints? Consider checking them for the constructed method anyway. #Closed

AlekseyTs · 2024-05-16T16:29:27Z

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

+            _factory.Syntax = receiver.Syntax;
+
+            var call = _factory.Call(null,
+                            _factory.WellKnownMethod(WellKnownMember.System_Runtime_CompilerServices_Unsafe__As_T)


System_Runtime_CompilerServices_Unsafe__As_T

We need to verify that we can rewrite this call across await. I.e. it should be possible to await in the argument list when we do this transform. Please make sure to add relevant tests. #Closed

Added AdjustReceiver_AwaitArgument. It errors. I left a PROTOTYPE comment as I didn't investigate closely whether we could make it work

AlekseyTs · 2024-05-16T16:34:58Z

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

+            }
+            else if (receiver.Type.IsValueType)
+            {
+                receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);


receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);

We need to verify that readonly semantics of the receiver is properly preserved (ref readonly/in parameters, readonly fields, this in readonly method, etc.). The receiver should be copied as appropriate. Please make sure to add relevant tests. #Closed

Added a set of readonly tests. In non-extension scenarios, the work of cloning the receiver is handled in EmitInstanceCallExpression (not in lowering). For extension scenarios, we assign the readonly parameter/field/this to a ref temp and the codegen for that assignment will handle that (in EmitAssignmentValue).

AlekseyTs · 2024-05-16T17:55:36Z

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

+            }
+            else if (receiver.Type.IsValueType)
+            {
+                receiver = MakeTemp(receiver, temps, effects, RefKind.Ref);


MakeTemp

Do we even need to do this capture? Could we simply pass the receiver by ref to Unsafe.As? Code gen might handle creation of an underlying temp automatically. #Closed

We do need this capture. I'd tried without and ran into an assertion ("passing args byref should not clone them into temps") in EmitArgument.

jcouv · 2024-05-21T00:55:37Z

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

@@ -906,6 +897,109 @@ static bool usesReceiver(BoundExpression argument)
            }
        }

+        private RefKind GetRefKindForCapturedReceiverTemp(BoundExpression rewrittenReceiver)


📝 This method is extracted, but the case for ref readonly parameter was added (was blowing up in StoreToTemp). Added a non-extensions repro

jcouv · 2024-05-21T01:00:20Z

        if (originalReceiver != rewrittenReceiver && rewrittenReceiver is BoundSequence sequence)

📝 this was causing problems (taking a rewritten receiver that is a sequence apart, but it's not the sequence this scenario had in mind) but the gymnastics were not necessary so simplified.

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_ObjectOrCollectionInitializerExpression.cs:238 in 3534070. [](commit_id = 3534070, deletion_comment = True)

AlekseyTs · 2024-05-22T15:31:12Z

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

            BoundLocal? receiverTemp = null;
            BoundAssignmentOperator? assignmentToTemp = null;

            if (captureReceiverMode != ReceiverCaptureMode.Default ||
-                (requiresInstanceReceiver && arguments.Any(a => usesReceiver(a))))
+                (requiresInstanceReceiver && arguments.Any(a => usesReceiver(a))) ||
+                wasReceiverAdjusted)


wasReceiverAdjusted

This looks suspicious. Why is this necessary? #Closed

The two "Transform" callers do a CanValueChangeBetweenReads check and pass ReceiveCaptureMode.Default when the value cannot change (or other capture modes when the value could change). For example, using a constant or this in a struct as the receiver.
It seems unnecessary to evaluate Unsafe.As twice in such cases, so we detect it and capture the temp anyways.
The extra temp did not appear to have any negative effect in the normal case (no compound assignment and no interpolation receiver capture), but I debated conditioning this only for those two "Transform" scnearios, by either adding a new capture mode and/or moving the CanValueChangeBetweenReads check here.

I am not comfortable with this approach. I think that the call sites that are trying to get some advantage/optimization by checking CanValueChangeBetweenReads should deal with that rather than everybody else "suffer". Adding a new capture mode would be fine too.

Also consider the following. An Unsafe.As is an intrinsic and its calls will be completely removed from the code by JIT, whereas the unnecessary (from semantics point of view) locals will likely stay. Therefore, I am not convinced that we need to worry about those extra calls.

AlekseyTs · 2024-05-22T15:40:13Z

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

+            // PROTOTYPE if we decide to allow readonly members in extensions, we'll need to revisit this
+            if (refKind is RefKind.RefReadOnly)
+            {
+                refKind = RefKind.Ref;


refKind = RefKind.Ref;

Consider adding a comment about what effect we are trying to have by making this adjustment. I.e. Readonly receivers must be always cloned since we do not support readonly extension members, using a mutable ref in this case will achieve this goal. #Closed

AlekseyTs · 2024-05-22T15:44:00Z

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

+            Debug.Assert(!receiver.Type.IsTypeParameter());
+            var refKind = GetRefKindForCapturedReceiverTemp(receiver, useNoneForReferenceType: true);
+            // PROTOTYPE if we decide to allow readonly members in extensions, we'll need to revisit this
+            if (refKind is RefKind.RefReadOnly)


refKind is RefKind.RefReadOnl

Should we also handle other flavors of readonly like RefKind.In. It is probably impossible to get RefKindExtensions.StrictIn here. Consider adding an assert then. #Closed

RedKind.In == RefKind.RefReadonly, so no extra handling needed. A test covers both.
Will add the assertion and a test for out

AlekseyTs · 2024-05-22T16:00:20Z

@jcouv It looks like there are legitimate test failures

AlekseyTs · 2024-05-22T16:11:51Z

    verifier.VerifyIL("<top-level-statements-entry-point>", """

Why are we dropping IL validation? #Closed

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:17517 in 8cdb571. [](commit_id = 8cdb571, deletion_comment = True)

AlekseyTs · 2024-05-22T16:40:59Z

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

@@ -3754,7 +3755,8 @@ static ParameterSymbol getCorrespondingParameter(in MemberAnalysisResult result,
                        case BoundInterpolatedStringArgumentPlaceholder.InstanceParameter:
                            Debug.Assert(receiver!.Type is not null);
                            refKind = RefKind.None;
-                            placeholderType = receiver.Type;
+                            bool useExtensionInstance = methodResult.Member.ContainingType.IsExtension && !receiver.Type.IsExtension;
+                            placeholderType = useExtensionInstance ? methodResult.Member.ContainingType : receiver.Type;


useExtensionInstance ? methodResult.Member.ContainingType : receiver.Type

Could we always use methodResult.Member.ContainingType and get rid of all complexity with calculating useExtensionInstance? #Closed

That breaks a number of existing interpolation tests (such as StructReceiver_Lvalue_04) because we need the placeholder type and the filler type to match. But for example, T (value) and ILogger (placeholder) don't match in that test.

AlekseyTs · 2024-05-22T16:43:12Z

src/Compilers/CSharp/Portable/CodeGen/EmitAddress.cs

@@ -68,7 +68,6 @@ private LocalDefinition EmitAddress(BoundExpression expression, AddressKind addr

                    if (expression.Type.IsValueType)
                    {
-
                        if (!HasHome(expression, addressKind))


if (!HasHome(expression, addressKind))

It doesn't look like there are any meaningful changes in this file. Consider reverting. #Closed

jcouv · 2024-05-22T16:44:25Z

    verifier.VerifyIL("<top-level-statements-entry-point>", """

CodeFlow is bungling this comment. It shows up in the middle of ExtensionInvocation_InaccessibleExtensionTypeMember_FallbackToExtensionMethod, so I'm not sure what it was meant for.
In the last commit, I removed some VerifyIL checks that I thought were not so useful (not unique/interesting enough) in an effort to trim down the PR a bit.

In reply to: 2125179623

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:17517 in 8cdb571. [](commit_id = 8cdb571, deletion_comment = True)

AlekseyTs · 2024-05-22T16:57:37Z

    verifier.VerifyIL("<top-level-statements-entry-point>", """

CodeFlow is bungling this comment. It shows up in the middle of ExtensionInvocation_InaccessibleExtensionTypeMember_FallbackToExtensionMethod, so I'm not sure what it was meant for.

If you look at the diff between commits 6 and 4, the comment is on the deleted IL verification right after the line 17473. If that IL was changed between the commits, I would find it useful to see the impact of the changes on it. However with validation deleted I cannot tell anything. I also see many more IL validation removed in the same diff. In general, given the nature of the changes, I find IL verification very useful/interesting. Therefore, if we have IL verification for the like scenario elsewhere, I think it is fine to get rid of duplication. Otherwise, I would keep verification for unique cases.

In reply to: 2125282065

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:17517 in 8cdb571. [](commit_id = 8cdb571, deletion_comment = True)

AlekseyTs · 2024-05-22T17:29:31Z

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

+            method.CheckConstraints(
+                new ConstraintsHelper.CheckConstraintsArgs(_compilation, _compilation.Conversions, temp.Syntax.GetLocation(), _diagnostics));
+
+            var call = _factory.Call(null, method, temp);


_factory.Call(null, method, temp);

I think we should pass useStrictArgumentRefKinds: true here. #Closed

AlekseyTs · 2024-05-22T17:44:36Z

Done with review pass (commit 6), tests are not looked at

AlekseyTs · 2024-05-23T15:58:15Z

implicit extension E for object

I think it would be interesting to test similar scenario with a struct type #Pending

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:45265 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T16:10:44Z

public C(int i) { System.Console.Write($"ctor({i}) "); }

I am not sure what we were trying to accomplish with this constructor. It looks like the value that we pass is not used in any meaningful way. #Closed

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:23422 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T16:18:04Z

public int Increment() => field += 1;

Not used? #Pending

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:41380 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T16:28:08Z

    get { System.Console.Write(this.field); return 0; }

When we are testing behavior around structures, I think we can benefit with some strengthening. We are verifying thar receiver's value is correct, but it would also be good to verify that receiver's location is correct. For example, this can be achieved by incrementing the field in each receiver and then also printing its value after the assignment. This way we can observe that the receiver refers to the expected location (no unexpected copies made) throughout the operation. For example, the code could look like this:

        var source = """
S s = new S();
s[s = new S() { field = 2 }] += 10;
System.Console.Write(s.field);

struct S
{
    public int field;
}

implicit extension E for S
{
    public int this[S s]
    {
        get { System.Console.Write(this.field++); return 0; }
        set { System.Console.Write(this.field++); }
    }
}
""";
```     #Resolved

---
Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:41309 in 0032ca5. [](commit_id = 0032ca52b6469af159f76628cb8db96bea541dec, deletion_comment = False)

AlekseyTs · 2024-05-23T16:33:11Z

            if (captureReceiverMode != ReceiverCaptureMode.Default)

Are we still hitting all the interesting code paths after the logic around wasReceiverAdjusted is removed?

In reply to: 2122988191

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs:667 in 8cdb571. [](commit_id = 8cdb571, deletion_comment = False)

AlekseyTs · 2024-05-23T16:50:03Z

cRef.M({{expression}}, cRef.Increment());

The scenario doesn't look very interesting when we are dealing with a class, but I could be missing something #Pending

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:43736 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T16:52:59Z

    s.Increment();

Why is this scenario interesting? Regardless of what we do with receiver, this call always operates on a copy because s is a value parameter. What were you trying to accomplish? #Pending

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:43993 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T16:56:47Z

sRef.M({{expression}}, sRef = ref otherS);

In tests like that, would it be more interesting to swap the arguments? Or do we cover that in other tests? #Pending

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:44033 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs

LGTM (commit 8)

jcouv · 2024-05-23T19:58:21Z

            if (captureReceiverMode != ReceiverCaptureMode.Default)

Yes, we're hitting:

compound branch indexer access that uses a receiver that is value type, reference type, and refs to both, and await argument on all four permutations
the interpolation instance capture branch with a receiver that is value type, reference type, and refs to both, and await argument for two permutations (ref S and ref C)

In reply to: 2127594065

Refers to: src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_Call.cs:667 in 8cdb571. [](commit_id = 8cdb571, deletion_comment = False)

jcouv · 2024-05-23T20:05:33Z

public C(int i) { System.Console.Write($"ctor({i}) "); }

This just adds a side-effect to the constructor which makes it obvious in the expectedOutput that the created instances were captured and we didn't evaluate the constructors more than we should.

In reply to: 2127533873

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:23422 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

jcouv · 2024-05-23T21:07:46Z

    get { System.Console.Write(this.field); return 0; }

this.field++ doesn't work yet. But I can add Increment() here as I did elsewhere

In reply to: 2127582150

Refers to: src/Compilers/CSharp/Test/Emit3/ExtensionTypeTests.cs:41309 in 0032ca5. [](commit_id = 0032ca5, deletion_comment = False)

AlekseyTs · 2024-05-23T21:15:30Z

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

+            tempsOpt.Add(temp.LocalSymbol);
+            Debug.Assert(temp.Type is not null);
+
+            MethodSymbol method = _factory.WellKnownMethod(WellKnownMember.System_Runtime_CompilerServices_Unsafe__As_T)


WellKnownMember.System_Runtime_CompilerServices_Unsafe__As_T

Are we testing scenario when this helper is not available? #Closed

Yes, AdjustReceiver_Invocation_ObjectCreation_MissingUnsafeAs

jcouv added Area-Compilers New Language Feature - Roles Roles labels May 16, 2024

jcouv self-assigned this May 16, 2024

dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label May 16, 2024

Adjust receiver in instance invocations

3534070

jcouv force-pushed the emit-instances2 branch from c1cfefa to 3534070 Compare May 16, 2024 00:41

jcouv requested review from jjonescz and AlekseyTs May 16, 2024 00:48

jcouv marked this pull request as ready for review May 16, 2024 00:48

jcouv requested a review from a team as a code owner May 16, 2024 00:48

jjonescz reviewed May 16, 2024

View reviewed changes

AlekseyTs reviewed May 16, 2024

View reviewed changes

Address feedback

c3fc74c

jcouv commented May 21, 2024

View reviewed changes

jcouv marked this pull request as draft May 21, 2024 01:14

Address feedback

21e206e

jcouv marked this pull request as ready for review May 21, 2024 02:41

jcouv requested review from jjonescz and AlekseyTs May 21, 2024 02:41

Tweaks

8cdb571

AlekseyTs reviewed May 22, 2024

View reviewed changes

jcouv added 2 commits May 22, 2024 13:26

Address feedback

7ff9d0f

Fix a test

0032ca5

jcouv requested review from AlekseyTs and jjonescz May 22, 2024 23:31

AlekseyTs approved these changes May 23, 2024

View reviewed changes

AlekseyTs reviewed May 23, 2024

View reviewed changes

Address feedback

c5bf70b

jcouv marked this pull request as draft May 24, 2024 00:01

Adjust receiver in instance invocations #73501

Are you sure you want to change the base?

Adjust receiver in instance invocations #73501

Conversation

jcouv commented May 16, 2024 • edited

jjonescz left a comment

Choose a reason for hiding this comment

jjonescz May 16, 2024 • edited by jcouv

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jjonescz May 16, 2024 • edited by jcouv

Choose a reason for hiding this comment

jcouv May 20, 2024 • edited

Choose a reason for hiding this comment

jjonescz May 16, 2024 • edited by jcouv

Choose a reason for hiding this comment

AlekseyTs commented May 16, 2024

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 16, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcouv May 21, 2024 • edited

Choose a reason for hiding this comment

jcouv commented May 21, 2024

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs commented May 22, 2024

AlekseyTs commented May 22, 2024 • edited

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

jcouv commented May 22, 2024

AlekseyTs commented May 22, 2024 • edited

AlekseyTs May 22, 2024 • edited

Choose a reason for hiding this comment

AlekseyTs commented May 22, 2024

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs commented May 23, 2024 • edited

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs commented May 23, 2024

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs commented May 23, 2024 • edited by jcouv

AlekseyTs left a comment

Choose a reason for hiding this comment

jcouv commented May 23, 2024

jcouv commented May 23, 2024

jcouv commented May 23, 2024

jcouv commented May 16, 2024 •

edited

jjonescz May 16, 2024 •

edited by jcouv

jjonescz May 16, 2024 •

edited by jcouv

jcouv May 20, 2024 •

edited

jjonescz May 16, 2024 •

edited by jcouv

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

AlekseyTs May 16, 2024 •

edited

jcouv May 21, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs commented May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs commented May 22, 2024 •

edited

AlekseyTs May 22, 2024 •

edited

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs commented May 23, 2024 •

edited

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs commented May 23, 2024 •

edited by jcouv

AlekseyTs May 23, 2024 •

edited