Lambda Invoke does not update arguments in the parameters array

[holdenmai]

Understandably, the Lambda class is essentially a wrapper around the parsed delegate due to the way we treat the parameter creation.

However, the way we currently implement the Invoke, the following fails.

        [Test]
        public void When_lambda_is_invoked_byref_parameters_are_updated_on_invoke()
        {
            var a = 1;
            var b = 2;
            var c = 3;
            var d = 4;
            var e = 5;

            var parameters = new[]{
                            new Parameter(nameof(a), typeof(int).MakeByRefType()),
                            new Parameter(nameof(b), typeof(int).MakeByRefType()),
                            new Parameter(nameof(c), typeof(int).MakeByRefType()),
                            new Parameter(nameof(d), typeof(int).MakeByRefType()),
                            new Parameter(nameof(e), typeof(int).MakeByRefType()),
                            };

            var args = new object[]
            {
                a,
                b,
                c,
                d,
                e
            };

            a += b *= c -= d /= e %= 3;

            var target = new Interpreter();

            var lambda = target.Parse("a = a + (b = b * (c = c - (d = d / (e = e % 3))))", parameters);

            Assert.AreEqual(a, lambda.Invoke(args));
            Assert.AreEqual(a, args[0]);
            Assert.AreEqual(b, args[1]);
            Assert.AreEqual(c, args[2]);
            Assert.AreEqual(d, args[3]);
            Assert.AreEqual(e, args[4]);
        }

If we had a reference to the actual delegate and performed a dynamic invoke on it with the args array, the Asserts would succeed.

[holdenmai]

I do have a fix for this ready local as it was blocking some of my testing of edge cases on collection initialization #250 , I would just like to confirm which path we want to go down.

Do we want to update the Invoke call or add an additional call that supports the ref parameters?

[davideicardi]

See also this comment: https://github.com/dynamicexpresso/DynamicExpresso/issues/251#issuecomment-1225648318

[holdenmai]

I agree from a security perspective with what you said.

In order for the underlying compiled Delegate to update the invoked objects, the parameter has to be declared as a ByRefType as follows typeof(int).MakeByRefType() So there is already a security layer being added from the part of the framework we are leveraging. We could also only do the final set on the output value if the related parameter is a ByRef type. Additionally this functionality can either be turned on as a setting (I know there are other settings to disable assign), or from a different Invoke call (like InvokeWithByRef).

Personally I think the cleanest option is to have a new Setting EnableByRefParameters that is off by default, and when it's enabled the ByRef still controls whether or not the ref/out parameter assignment can be accomplished.

I think there would also be some merit in allowing this functionality in some way since there is a unit test specifically set up to allow setting to a parameter (Can_assign_a_parameter)

[holdenmai]

Realized this is very similar to #31